diff --git a/app/Jobs/HostPathCloneJob.php b/app/Jobs/HostPathCloneJob.php index 1a7300b880..d308de7c34 100644 --- a/app/Jobs/HostPathCloneJob.php +++ b/app/Jobs/HostPathCloneJob.php @@ -63,10 +63,8 @@ class HostPathCloneJob implements ShouldBeEncrypted, ShouldQueue { $archiveName = 'hostpath-data.tar.gz'; $token = Str::uuid()->toString(); - $sourceCloneDir = $this->cloneArchiveDirectory($this->sourceServer, "hostpath-{$token}"); - $targetCloneDir = $this->cloneArchiveDirectory($this->targetServer, "hostpath-{$token}"); - $srcDir = escapeshellarg($sourceCloneDir); - $tgtDir = escapeshellarg($targetCloneDir); + $sourceCloneDir = null; + $targetCloneDir = null; $srcPath = escapeshellarg($this->sourcePath); $tgtPath = escapeshellarg($this->targetPath); $tgtParent = escapeshellarg(dirname($this->targetPath)); @@ -76,13 +74,15 @@ class HostPathCloneJob implements ShouldBeEncrypted, ShouldQueue try { File::ensureDirectoryExists($localTempDir, 0755); + $sourceCloneDir = $this->createCloneArchiveDirectory($this->sourceServer, "hostpath-{$token}"); + $srcDir = escapeshellarg($sourceCloneDir); instant_remote_process([ - ...$this->prepareCloneArchiveDirectory($this->sourceServer, $sourceCloneDir), "test -e {$srcPath}", "docker run --rm -v {$srcPath}:/source:ro -v {$srcDir}:/clone alpine sh -c 'cd /source && tar czf /clone/{$archiveName} .'", ], $this->sourceServer); - instant_remote_process($this->prepareCloneArchiveDirectory($this->targetServer, $targetCloneDir), $this->targetServer); + $targetCloneDir = $this->createCloneArchiveDirectory($this->targetServer, "hostpath-{$token}"); + $tgtDir = escapeshellarg($targetCloneDir); instant_scp_from_server( "{$sourceCloneDir}/{$archiveName}", @@ -111,17 +111,8 @@ class HostPathCloneJob implements ShouldBeEncrypted, ShouldQueue \Log::warning('Failed to clean up local host-path clone directory: '.$e->getMessage()); } - try { - instant_remote_process(["rm -rf {$srcDir}"], $this->sourceServer, false); - } catch (\Exception $e) { - \Log::warning('Failed to clean up source host-path clone directory: '.$e->getMessage()); - } - - try { - instant_remote_process(["rm -rf {$tgtDir}"], $this->targetServer, false); - } catch (\Exception $e) { - \Log::warning('Failed to clean up target host-path clone directory: '.$e->getMessage()); - } + $this->removeCloneArchiveDirectory($this->sourceServer, $sourceCloneDir); + $this->removeCloneArchiveDirectory($this->targetServer, $targetCloneDir); } } } diff --git a/app/Jobs/VolumeCloneJob.php b/app/Jobs/VolumeCloneJob.php index d43705bc5a..86ddcabe86 100644 --- a/app/Jobs/VolumeCloneJob.php +++ b/app/Jobs/VolumeCloneJob.php @@ -54,32 +54,53 @@ class VolumeCloneJob implements ShouldBeEncrypted, ShouldQueue return; } + $sourceConfiguration = $this->sourceVolumeConfiguration(); + $createTargetVolume = $this->createTargetVolumeCommand($sourceConfiguration); + + if ($this->isBindVolume($sourceConfiguration)) { + instant_remote_process([$createTargetVolume], $this->sourceServer); + $this->logSkippedBindVolumeCopy($sourceConfiguration); + + return; + } + instant_remote_process([ - "docker volume create {$tgtVol}", + $createTargetVolume, "docker run --rm -v {$srcVol}:/source -v {$tgtVol}:/target alpine sh -c 'cp -a /source/. /target/ && chown -R 1000:1000 /target'", ], $this->sourceServer); + $this->rememberTargetVolumeWithoutDriverOptions($sourceConfiguration); } protected function cloneRemoteVolume(): void { $srcVol = escapeshellarg($this->sourceVolume); $tgtVol = escapeshellarg($this->targetVolume); - $sourceCloneDir = $this->cloneArchiveDirectory($this->sourceServer, $this->sourceVolume); - $targetCloneDir = $this->cloneArchiveDirectory($this->targetServer, $this->targetVolume); - $srcDir = escapeshellarg($sourceCloneDir); - $tgtDir = escapeshellarg($targetCloneDir); + $sourceConfiguration = $this->sourceVolumeConfiguration(); + $createTargetVolume = $this->createTargetVolumeCommand($sourceConfiguration); + + if ($this->isBindVolume($sourceConfiguration)) { + instant_remote_process([$createTargetVolume], $this->targetServer); + $this->logSkippedBindVolumeCopy($sourceConfiguration); + + return; + } + + $sourceCloneDir = null; + $targetCloneDir = null; $localTempDir = storage_path('app/tmp/volume-clones/'.Str::uuid()->toString()); $localArchive = $localTempDir.'/volume-data.tar.gz'; try { File::ensureDirectoryExists($localTempDir, 0755); + $sourceCloneDir = $this->createCloneArchiveDirectory($this->sourceServer, $this->sourceVolume); + $srcDir = escapeshellarg($sourceCloneDir); instant_remote_process([ - ...$this->prepareCloneArchiveDirectory($this->sourceServer, $sourceCloneDir), "docker run --rm -v {$srcVol}:/source -v {$srcDir}:/clone alpine sh -c 'cd /source && tar czf /clone/volume-data.tar.gz .'", ], $this->sourceServer); - instant_remote_process($this->prepareCloneArchiveDirectory($this->targetServer, $targetCloneDir), $this->targetServer); + $targetCloneDir = $this->createCloneArchiveDirectory($this->targetServer, $this->targetVolume); + $tgtDir = escapeshellarg($targetCloneDir); // Coolify host is the intermediary: download from source, upload to target. instant_scp_from_server( @@ -95,9 +116,10 @@ class VolumeCloneJob implements ShouldBeEncrypted, ShouldQueue ); instant_remote_process([ - "docker volume create {$tgtVol}", + $createTargetVolume, "docker run --rm -v {$tgtVol}:/target -v {$tgtDir}:/clone alpine sh -c 'cd /target && tar xzf /clone/volume-data.tar.gz && chown -R 1000:1000 /target'", ], $this->targetServer); + $this->rememberTargetVolumeWithoutDriverOptions($sourceConfiguration); } catch (\Exception $e) { \Log::error("Failed to clone volume {$this->sourceVolume} to {$this->targetVolume}: ".$e->getMessage()); throw $e; @@ -108,23 +130,99 @@ class VolumeCloneJob implements ShouldBeEncrypted, ShouldQueue \Log::warning('Failed to clean up local volume clone directory: '.$e->getMessage()); } - try { - instant_remote_process([ - "rm -rf {$srcDir}", - ], $this->sourceServer, false); - } catch (\Exception $e) { - \Log::warning('Failed to clean up source server clone directory: '.$e->getMessage()); - } - - try { - if ($this->targetServer) { - instant_remote_process([ - "rm -rf {$tgtDir}", - ], $this->targetServer, false); - } - } catch (\Exception $e) { - \Log::warning('Failed to clean up target server clone directory: '.$e->getMessage()); - } + $this->removeCloneArchiveDirectory($this->sourceServer, $sourceCloneDir); + $this->removeCloneArchiveDirectory($this->targetServer, $targetCloneDir); } } + + /** + * The driver and driver options of the source volume. A missing source volume gives a plain + * `local` volume, as Docker creates an empty source volume for the copy. + * + * @return array{driver: string, options: array} + */ + protected function sourceVolumeConfiguration(): array + { + $output = instant_remote_process([ + 'docker volume inspect '.escapeshellarg($this->sourceVolume)." 2>/dev/null || echo '[]'", + ], $this->sourceServer); + $inspected = json_decode((string) $output, true); + $volume = is_array($inspected) && is_array($inspected[0] ?? null) ? $inspected[0] : []; + + $driver = data_get($volume, 'Driver'); + $options = []; + foreach ((array) data_get($volume, 'Options', []) as $key => $value) { + if (is_string($key) && $key !== '' && is_scalar($value)) { + $options[$key] = (string) $value; + } + } + + return [ + 'driver' => is_string($driver) && $driver !== '' ? $driver : 'local', + 'options' => $options, + ]; + } + + /** + * Creates the target volume like the source volume. The Compose file of the copy declares the same + * `driver` and `driver_opts`, and Docker Compose does not change the options of an existing volume. + * One `sh -c` line, so the non-root sudo parser only puts sudo in front of it and never changes + * an option value. + * + * @param array{driver: string, options: array} $configuration + */ + protected function createTargetVolumeCommand(array $configuration): string + { + $command = 'docker volume create'; + if ($configuration['driver'] !== 'local') { + $command .= ' --driver '.escapeshellarg($configuration['driver']); + } + foreach ($configuration['options'] as $key => $value) { + $command .= ' --opt '.escapeshellarg("{$key}={$value}"); + } + $command .= ' '.escapeshellarg($this->targetVolume); + + return 'sh -c '.escapeshellarg($command); + } + + /** + * A `local` volume with `type: none` and `o: bind` is a host folder. Copying the data would write + * into that folder (on the same server, the folder of the source volume itself), so the job only + * creates the volume. + * + * @param array{driver: string, options: array} $configuration + */ + protected function isBindVolume(array $configuration): bool + { + return $configuration['driver'] === 'local' + && ($configuration['options']['type'] ?? null) === 'none' + && in_array('bind', array_map('trim', explode(',', $configuration['options']['o'] ?? '')), true); + } + + /** + * @param array{driver: string, options: array} $configuration + */ + protected function logSkippedBindVolumeCopy(array $configuration): void + { + $device = $configuration['options']['device'] ?? 'unknown'; + \Log::info("Volume {$this->sourceVolume} is a bind mount of the host folder {$device}. Created {$this->targetVolume} with the same options and did not copy the data."); + } + + /** + * A target volume without driver options gets the name-only Compose declaration, like any volume + * that Docker created without the options (see composeRenamedVolumeDeclarationFor()). + * + * @param array{driver: string, options: array} $configuration + */ + protected function rememberTargetVolumeWithoutDriverOptions(array $configuration): void + { + if ($configuration['driver'] !== 'local' || $configuration['options'] !== []) { + return; + } + if ($this->persistentVolume->name !== $this->targetVolume || $this->persistentVolume->ignores_compose_driver_options) { + return; + } + + $this->persistentVolume->forceFill(['ignores_compose_driver_options' => true])->saveQuietly(); + } } diff --git a/app/Models/LocalFileVolume.php b/app/Models/LocalFileVolume.php index 08f8f870d5..8e758566e6 100644 --- a/app/Models/LocalFileVolume.php +++ b/app/Models/LocalFileVolume.php @@ -10,6 +10,7 @@ use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Relations\MorphMany; use Illuminate\Database\Eloquent\Relations\MorphTo; use Illuminate\Support\Collection; +use Illuminate\Support\Stringable; use Symfony\Component\Yaml\Yaml; class LocalFileVolume extends BaseModel @@ -170,11 +171,7 @@ class LocalFileVolume extends BaseModel $server = $this->resource->destination->server; } $commands = collect([]); - $path = data_get_str($this, 'fs_path'); - if ($path->startsWith('.')) { - $path = $path->after('.'); - $path = $workdir.$path; - } + $path = $this->resolvedFsPath($workdir); if (! $this->isAdminControlledComposeMount()) { $path = str(confinePathToBase($workdir, $path->value(), 'storage path')); @@ -254,11 +251,7 @@ class LocalFileVolume extends BaseModel $isService = data_get($this->resource, 'service'); $workdir = $isService ? $this->resource->service->workdir() : $this->resource->workdir(); $commands = collect([]); - $path = data_get_str($this, 'fs_path'); - if ($path->startsWith('.')) { - $path = $path->after('.'); - $path = $workdir.$path; - } + $path = $this->resolvedFsPath($workdir); if (! $this->isAdminControlledComposeMount()) { $path = str(confinePathToBase($workdir, $path->value(), 'storage path')); @@ -302,20 +295,9 @@ class LocalFileVolume extends BaseModel $commands = collect([]); $escapedWorkdir = escapeshellarg($workdir); - if ($this->is_directory) { - // Validate fs_path early before any shell interpolation - validateShellSafePath($this->fs_path, 'storage path'); - $escapedFsPath = escapeshellarg($this->fs_path); - $commands->push("mkdir -p {$escapedFsPath} > /dev/null 2>&1 || true"); - $commands->push("mkdir -p {$escapedWorkdir} > /dev/null 2>&1 || true"); - } - $path = data_get_str($this, 'fs_path'); + $path = $this->resolvedFsPath($workdir); $content = data_get($this, 'content'); $writesContent = $this->writesContentOnServer(); - if ($path->startsWith('.')) { - $path = $path->after('.'); - $path = $workdir.$path; - } if ($writesContent) { $path = str($this->confinedContentPath($path->value(), $server)); @@ -324,9 +306,14 @@ class LocalFileVolume extends BaseModel $this->assertRemotePathIsConfined($workdir, $path->value(), $server); } - $pathForParentDirectory = $writesContent ? $path : str($this->fs_path); - if ($pathForParentDirectory->startsWith('.') || $pathForParentDirectory->startsWith('/') || $pathForParentDirectory->startsWith('~')) { - $parent_dir = $pathForParentDirectory->beforeLast('/'); + if ($this->is_directory) { + validateShellSafePath($path, 'storage path'); + $commands->push('mkdir -p '.escapeshellarg($path).' > /dev/null 2>&1 || true'); + $commands->push("mkdir -p {$escapedWorkdir} > /dev/null 2>&1 || true"); + } + + if ($path->startsWith('/') || $path->startsWith('~')) { + $parent_dir = $path->beforeLast('/'); if ($parent_dir != '') { $escapedParentDir = escapeshellarg($parent_dir); $commands->push("mkdir -p {$escapedParentDir} > /dev/null 2>&1 || true"); @@ -432,12 +419,27 @@ class LocalFileVolume extends BaseModel */ public function contentPathOnServer(): string { - $path = (string) $this->fs_path; + return $this->localConfinedContentPath($this->resolvedFsPath($this->ownerResource()->workdir())->value())[1]; + } + + /** + * The host path of this mount. Like a relative bind source in Compose, a relative path is inside + * the resource directory. Remote commands run in the home directory of the SSH user, so they + * must never get the relative path. A `~` path stays in the home directory. + */ + public function resolvedFsPath(string $workdir): Stringable + { + $path = trim((string) $this->fs_path); + if (str_starts_with($path, '/') || str_starts_with($path, '~')) { + return str($path); + } if (str_starts_with($path, '.')) { - $path = $this->ownerResource()->workdir().substr($path, 1); + $path = substr($path, 1); } - return $this->localConfinedContentPath($path)[1]; + $path = ltrim($path, '/'); + + return str(rtrim($workdir, '/').($path === '' ? '' : '/'.$path)); } /** diff --git a/app/Traits/StagesCloneArchives.php b/app/Traits/StagesCloneArchives.php index 68e08798a6..9bbdaeb5a0 100644 --- a/app/Traits/StagesCloneArchives.php +++ b/app/Traits/StagesCloneArchives.php @@ -3,39 +3,70 @@ namespace App\Traits; use App\Models\Server; +use RuntimeException; +use Throwable; /** * Clone jobs move an archive between servers with scp, which reads and writes as the SSH user. * On the Coolify host, /data/coolify is closed to a non-root SSH user, so the archive goes through - * a directory in /var/tmp (on disk, not tmpfs) that only that user and root can open. + * a directory in /var/tmp (on disk, not tmpfs) that only that user and root can open. /var/tmp is + * writable by every local user, so the directory gets a random name from mktemp: a fixed name could + * be created beforehand, or be a symlink that the sudo chown and chmod would follow. */ trait StagesCloneArchives { protected string $cloneDir = '/data/coolify/clone'; - protected function cloneArchiveDirectory(Server $server, string $name): string - { - return ($server->isNonRoot() ? '/var/tmp/coolify-clone' : $this->cloneDir)."/{$name}"; - } - /** - * @return list + * Creates the directory for the clone archive on the server and returns its path. */ - protected function prepareCloneArchiveDirectory(Server $server, string $directory): array + protected function createCloneArchiveDirectory(Server $server, string $name): string { - $escapedDirectory = escapeshellarg($directory); - - if ($server->isNonRoot()) { - return [ + if (! $server->isNonRoot()) { + $directory = "{$this->cloneDir}/{$name}"; + $escapedDirectory = escapeshellarg($directory); + instant_remote_process([ "mkdir -p {$escapedDirectory}", - 'chown '.escapeshellarg($server->user)." {$escapedDirectory}", - "chmod 700 {$escapedDirectory}", - ]; + "chmod 777 {$escapedDirectory}", + ], $server); + + return $directory; } - return [ - "mkdir -p {$escapedDirectory}", - "chmod 777 {$escapedDirectory}", - ]; + // The sudo parser runs mktemp as root, so the new directory belongs to root with mode 700. + // mktemp creates a new directory and never reuses an existing path or symlink. BusyBox + // (Alpine) randomizes only the last 6 X, which still matches the pattern below. + $directory = trim((string) instant_remote_process(['mktemp -d /var/tmp/coolify-clone.XXXXXXXXXX'], $server)); + if (preg_match('#\A/var/tmp/coolify-clone\.[A-Za-z0-9]{10}\z#', $directory) !== 1) { + throw new RuntimeException('Could not create a temporary clone directory on server '.$server->name.'.'); + } + + $escapedDirectory = escapeshellarg($directory); + try { + // scp reads and writes the archive as the SSH user. + instant_remote_process([ + 'chown '.escapeshellarg($server->user)." {$escapedDirectory}", + "chmod 700 {$escapedDirectory}", + ], $server); + } catch (Throwable $e) { + $this->removeCloneArchiveDirectory($server, $directory); + + throw $e; + } + + return $directory; + } + + protected function removeCloneArchiveDirectory(?Server $server, ?string $directory): void + { + if (! $server || $directory === null) { + return; + } + + try { + instant_remote_process(['rm -rf '.escapeshellarg($directory)], $server, false); + } catch (Throwable $e) { + \Log::warning("Failed to clean up clone directory {$directory} on server {$server->name}: ".$e->getMessage()); + } } } diff --git a/tests/Feature/CloneArchiveDirectoryTest.php b/tests/Feature/CloneArchiveDirectoryTest.php new file mode 100644 index 0000000000..92b1b2df3c --- /dev/null +++ b/tests/Feature/CloneArchiveDirectoryTest.php @@ -0,0 +1,155 @@ + 0]); + config(['app.maintenance.store' => 'array', 'constants.ssh.mux_enabled' => false]); + + $team = Team::factory()->create(); + $privateKey = PrivateKey::factory()->create(['team_id' => $team->id]); + $this->makeServer = fn (string $user) => Server::factory()->create([ + 'team_id' => $team->id, + 'user' => $user, + 'private_key_id' => $privateKey->id, + ]); + $this->volume = LocalPersistentVolume::forceCreate([ + 'name' => 'clone-target', + 'mount_path' => '/data', + 'resource_type' => Application::class, + 'resource_id' => 1, + ]); + $this->mktempDirectories = []; +}); + +/** + * Fakes the servers: mktemp prints a new random directory, `docker volume inspect` prints nothing, + * and scp fails when $failScp is set. + */ +function fakeCloneArchiveServers(object $test, bool $failScp = false): void +{ + Process::fake(function ($process) use ($test, $failScp) { + if (str_contains($process->command, 'mktemp -d')) { + $directory = '/var/tmp/coolify-clone.'.Str::random(10); + $test->mktempDirectories[] = $directory; + + return Process::result(output: $directory); + } + if ($failScp && str_contains($process->command, ' scp ')) { + return Process::result(output: '', errorOutput: 'scp failed', exitCode: 1); + } + + return Process::result(output: str_contains($process->command, 'docker volume inspect') ? '[]' : ''); + }); +} + +/** + * @return list + */ +function cloneArchiveCommands(): array +{ + $commands = []; + Process::assertRan(function ($process) use (&$commands) { + $commands[] = $process->command; + + return true; + }); + + return $commands; +} + +dataset('clone jobs', [ + 'volume clone' => [fn (Server $source, Server $target) => new VolumeCloneJob('clone-source', 'clone-target', $source, $target, test()->volume), 'volume-data.tar.gz'], + 'host path clone' => [fn (Server $source, Server $target) => new HostPathCloneJob('/srv/app-data', '/srv/app-copy', $source, $target), 'hostpath-data.tar.gz'], +]); + +it('stages a non-root clone in random mktemp directories with mode 700 and removes them', function (Closure $makeJob, string $archive) { + $source = ($this->makeServer)('cooluser'); + $target = ($this->makeServer)('cooluser'); + fakeCloneArchiveServers($this); + + $makeJob($source, $target)->handle(); + + $commands = implode("\n", cloneArchiveCommands()); + [$sourceDirectory, $targetDirectory] = $this->mktempDirectories; + + expect($this->mktempDirectories)->toHaveCount(2) + ->and($sourceDirectory)->not->toBe($targetDirectory) + ->and(substr_count($commands, 'sudo mktemp -d /var/tmp/coolify-clone.XXXXXXXXXX'))->toBe(2) + ->and($commands)->not->toContain('/var/tmp/coolify-clone/') + ->and($commands)->not->toContain('/data/coolify/clone'); + + foreach ([$sourceDirectory, $targetDirectory] as $directory) { + $escaped = escapeshellarg($directory); + expect($commands)->toContain("sudo chown 'cooluser' {$escaped}") + ->and($commands)->toContain("sudo chmod 700 {$escaped}") + ->and($commands)->toContain("{$directory}/{$archive}") + ->and($commands)->toContain("sudo rm -rf {$escaped}"); + } + // The sudo parser wraps each docker run line in `sudo bash -c '...'`. + expect($commands)->toContain("-v '\\''{$sourceDirectory}'\\'':/clone") + ->and($commands)->toContain("-v '\\''{$targetDirectory}'\\'':/clone"); +})->with('clone jobs'); + +it('removes the random non-root clone directories when the copy fails', function (Closure $makeJob) { + $source = ($this->makeServer)('cooluser'); + $target = ($this->makeServer)('cooluser'); + fakeCloneArchiveServers($this, failScp: true); + + expect(fn () => $makeJob($source, $target)->handle())->toThrow(RuntimeException::class); + + $commands = implode("\n", cloneArchiveCommands()); + expect($this->mktempDirectories)->toHaveCount(2); + foreach ($this->mktempDirectories as $directory) { + expect($commands)->toContain('sudo rm -rf '.escapeshellarg($directory)); + } + expect($commands)->not->toContain('tar xzf'); +})->with('clone jobs'); + +it('keeps the /data/coolify/clone directory for a root SSH user and removes it', function (Closure $makeJob, string $archive, bool $fail) { + $source = ($this->makeServer)('root'); + $target = ($this->makeServer)('root'); + fakeCloneArchiveServers($this, failScp: $fail); + + $run = fn () => $makeJob($source, $target)->handle(); + if ($fail) { + expect($run)->toThrow(RuntimeException::class); + } else { + $run(); + } + + $commands = cloneArchiveCommands(); + $all = implode("\n", $commands); + $directories = []; + foreach ($commands as $command) { + if (preg_match("#mkdir -p '(/data/coolify/clone/[^']+)'#", $command, $matches)) { + $directories[] = $matches[1]; + } + } + + expect($this->mktempDirectories)->toBe([]) + ->and($all)->not->toContain('/var/tmp') + ->and($directories)->toHaveCount(2); + foreach ($directories as $directory) { + expect($all)->toContain('chmod 777 '.escapeshellarg($directory)) + ->and($all)->toContain('rm -rf '.escapeshellarg($directory)); + } +})->with('clone jobs')->with(['success' => false, 'failure' => true]); diff --git a/tests/Feature/LocalFileVolumeRelativePathTest.php b/tests/Feature/LocalFileVolumeRelativePathTest.php new file mode 100644 index 0000000000..a60bcddbf5 --- /dev/null +++ b/tests/Feature/LocalFileVolumeRelativePathTest.php @@ -0,0 +1,123 @@ + 0]); + config([ + 'app.maintenance.store' => 'array', + 'constants.ssh.mux_enabled' => false, + ]); + + $team = Team::factory()->create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'private_key_id' => PrivateKey::factory()->create(['team_id' => $team->id])->id, + ]); + $destination = StandaloneDocker::query()->where('server_id', $server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + + $this->service = Service::factory()->create([ + 'environment_id' => $environment->id, + 'server_id' => $server->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + 'docker_compose_raw' => "services:\n app:\n image: nginx:alpine\n", + ]); + $this->serviceApplication = ServiceApplication::create(['name' => 'app', 'service_id' => $this->service->id]); + + Process::fake(fn ($process) => Process::result(output: match (true) { + str_contains($process->command, 'readlink -f') => 'OK', + str_contains($process->command, 'test -') => 'NOK', + default => '', + })); +}); + +function relativePathFileVolume(string $fsPath, bool $isDirectory, ?string $content = null): LocalFileVolume +{ + return LocalFileVolume::create([ + 'fs_path' => $fsPath, + 'mount_path' => '/data', + 'content' => $content, + 'is_directory' => $isDirectory, + 'resource_id' => test()->serviceApplication->id, + 'resource_type' => test()->serviceApplication->getMorphClass(), + ])->fresh(); +} + +/** + * Remote commands run from the SSH user's home directory, so a relative path in a command + * would create files and directories in `~` instead of the resource directory. + */ +function assertNoRelativeRemotePath(): void +{ + Process::assertDidntRun(fn ($process) => str_contains($process->command, "'./")); +} + +test('a relative directory path is created inside the resource directory, not in the home directory', function () { + $workdir = $this->service->workdir(); + relativePathFileVolume('./data/sub', isDirectory: true)->saveStorageOnServer(); + + Process::assertRan(fn ($process) => str_contains($process->command, "mkdir -p '{$workdir}/data/sub'")); + Process::assertRan(fn ($process) => str_contains($process->command, "mkdir -p '{$workdir}/data'")); + assertNoRelativeRemotePath(); +}); + +test('a relative file path without content gets its parent directory inside the resource directory', function () { + $workdir = $this->service->workdir(); + relativePathFileVolume('./config/app.conf', isDirectory: false)->saveStorageOnServer(); + + Process::assertRan(fn ($process) => str_contains($process->command, "mkdir -p '{$workdir}/config'")); + Process::assertRan(fn ($process) => str_contains($process->command, "touch '{$workdir}/config/app.conf'")); + assertNoRelativeRemotePath(); +}); + +test('a relative file path with content is written inside the resource directory', function () { + $workdir = $this->service->workdir(); + relativePathFileVolume('./config/app.conf', isDirectory: false, content: 'listen 8080;')->saveStorageOnServer(); + + Process::assertRan(fn ($process) => str_contains($process->command, "| base64 -d | tee '{$workdir}/config/app.conf' > /dev/null")); + assertNoRelativeRemotePath(); +}); + +test('an absolute directory path keeps working unchanged', function () { + $workdir = $this->service->workdir(); + relativePathFileVolume($workdir.'/data', isDirectory: true)->saveStorageOnServer(); + + Process::assertRan(fn ($process) => str_contains($process->command, "mkdir -p '{$workdir}/data'")); + assertNoRelativeRemotePath(); +}); + +test('deleting a relative path removes it from the resource directory', function () { + Process::fake(fn ($process) => Process::result(output: match (true) { + str_contains($process->command, 'readlink -f') => 'OK', + str_contains($process->command, 'test -f') => 'OK', + default => 'NOK', + })); + $workdir = $this->service->workdir(); + relativePathFileVolume('./config/app.conf', isDirectory: false)->deleteStorageOnServer(); + + Process::assertRan(fn ($process) => str_contains($process->command, "rm -rf '{$workdir}/config/app.conf'")); + assertNoRelativeRemotePath(); +}); + +test('the content path on the server resolves a relative path against the resource directory', function () { + expect(relativePathFileVolume('./config/app.conf', isDirectory: false, content: 'x')->contentPathOnServer()) + ->toBe($this->service->workdir().'/config/app.conf'); +}); diff --git a/tests/Feature/NonRootDataDirectoryCommandsTest.php b/tests/Feature/NonRootDataDirectoryCommandsTest.php index def281e0b2..2849854e29 100644 --- a/tests/Feature/NonRootDataDirectoryCommandsTest.php +++ b/tests/Feature/NonRootDataDirectoryCommandsTest.php @@ -113,27 +113,57 @@ it('writes a database dump and its redirect in one root shell', function () { ]); }); -it('stages clone archives outside /data/coolify only for a non-root SSH user', function () { +it('stages clone archives in a random mktemp directory only for a non-root SSH user', function () { $stager = new class { use StagesCloneArchives { - cloneArchiveDirectory as public; - prepareCloneArchiveDirectory as public; + createCloneArchiveDirectory as public; } }; $rootServer = Server::factory()->create(['team_id' => $this->server->team_id, 'user' => 'root', 'private_key_id' => $this->server->private_key_id]); + Process::fake(fn ($process) => Process::result( + output: str_contains($process->command, 'mktemp -d') ? '/var/tmp/coolify-clone.Ab3dE6gH9k' : '', + )); - $nonRootDirectory = $stager->cloneArchiveDirectory($this->server, 'volume-a'); + $nonRootDirectory = $stager->createCloneArchiveDirectory($this->server, 'volume-a'); + $rootDirectory = $stager->createCloneArchiveDirectory($rootServer, 'volume-a'); - expect($nonRootDirectory)->toBe('/var/tmp/coolify-clone/volume-a') - ->and($stager->prepareCloneArchiveDirectory($this->server, $nonRootDirectory))->toBe([ - "mkdir -p '/var/tmp/coolify-clone/volume-a'", - "chown 'cooluser' '/var/tmp/coolify-clone/volume-a'", - "chmod 700 '/var/tmp/coolify-clone/volume-a'", - ]) - ->and($stager->cloneArchiveDirectory($rootServer, 'volume-a'))->toBe('/data/coolify/clone/volume-a') - ->and($stager->prepareCloneArchiveDirectory($rootServer, '/data/coolify/clone/volume-a'))->toBe([ - "mkdir -p '/data/coolify/clone/volume-a'", - "chmod 777 '/data/coolify/clone/volume-a'", - ]); + $commands = []; + Process::assertRan(function ($process) use (&$commands) { + $commands[] = $process->command; + + return true; + }); + $commands = implode("\n", $commands); + + expect($nonRootDirectory)->toBe('/var/tmp/coolify-clone.Ab3dE6gH9k') + ->and($commands)->toContain('sudo mktemp -d /var/tmp/coolify-clone.XXXXXXXXXX') + ->and($commands)->toContain("sudo chown 'cooluser' '/var/tmp/coolify-clone.Ab3dE6gH9k'") + ->and($commands)->toContain("sudo chmod 700 '/var/tmp/coolify-clone.Ab3dE6gH9k'") + ->and($commands)->not->toContain('/var/tmp/coolify-clone/') + ->and($rootDirectory)->toBe('/data/coolify/clone/volume-a') + ->and($commands)->toContain("mkdir -p '/data/coolify/clone/volume-a'") + ->and($commands)->toContain("chmod 777 '/data/coolify/clone/volume-a'"); }); + +it('rejects a clone archive directory that mktemp did not create', function (string $output) { + $stager = new class + { + use StagesCloneArchives { + createCloneArchiveDirectory as public; + } + }; + Process::fake(fn ($process) => Process::result( + output: str_contains($process->command, 'mktemp -d') ? $output : '', + )); + + expect(fn () => $stager->createCloneArchiveDirectory($this->server, 'volume-a'))->toThrow(RuntimeException::class); + Process::assertNotRan(fn ($process) => str_contains($process->command, 'chown') || str_contains($process->command, 'chmod')); +})->with([ + 'empty' => [''], + 'predictable path' => ['/var/tmp/coolify-clone/volume-a'], + 'traversal' => ['/var/tmp/coolify-clone.Ab3dE6gH9k/../../etc'], + 'other directory' => ['/tmp/coolify-clone.Ab3dE6gH9k'], + 'extra line' => ["/var/tmp/coolify-clone.Ab3dE6gH9k\n/etc"], + 'shell characters' => ['/var/tmp/coolify-clone.Ab3d$(id)k'], +]); diff --git a/tests/Feature/VolumeCloneJobTest.php b/tests/Feature/VolumeCloneJobTest.php new file mode 100644 index 0000000000..e46eb24c92 --- /dev/null +++ b/tests/Feature/VolumeCloneJobTest.php @@ -0,0 +1,186 @@ + 0]); + config(['app.maintenance.store' => 'array', 'constants.ssh.mux_enabled' => false]); + + $this->team = Team::factory()->create(); + $this->privateKey = PrivateKey::factory()->create(['team_id' => $this->team->id]); + $this->server = Server::factory()->create([ + 'team_id' => $this->team->id, + 'user' => 'root', + 'private_key_id' => $this->privateKey->id, + ]); + $this->volume = LocalPersistentVolume::forceCreate([ + 'name' => 'clone-target', + 'mount_path' => '/data', + 'resource_type' => Application::class, + 'resource_id' => 1, + 'ignores_compose_driver_options' => false, + ]); +}); + +/** + * Fakes the server: `docker volume inspect` prints the given source volume, everything else succeeds. + * + * @param array|null $sourceVolume + */ +function fakeVolumeCloneServer(?array $sourceVolume): void +{ + Process::fake(fn ($process) => Process::result( + output: str_contains($process->command, 'docker volume inspect') + ? json_encode($sourceVolume === null ? [] : [$sourceVolume]) + : '', + )); +} + +function volumeCloneCommands(): string +{ + $commands = []; + Process::assertRan(function ($process) use (&$commands) { + $commands[] = $process->command; + + return true; + }); + + return implode("\n", $commands); +} + +test('a local clone creates the target volume with the driver options of the source volume', function () { + fakeVolumeCloneServer([ + 'Name' => 'clone-source', + 'Driver' => 'local', + 'Options' => ['type' => 'nfs', 'o' => 'addr=10.0.0.5,rw,nfsvers=4', 'device' => ':/exports/data'], + ]); + + (new VolumeCloneJob('clone-source', 'clone-target', $this->server, $this->server, $this->volume))->handle(); + + $commands = volumeCloneCommands(); + expect($commands)->toContain('docker volume inspect '.escapeshellarg('clone-source')) + ->and($commands)->toContain(escapeshellarg("docker volume create --opt 'type=nfs' --opt 'o=addr=10.0.0.5,rw,nfsvers=4' --opt 'device=:/exports/data' 'clone-target'")) + ->and($commands)->toContain('cp -a /source/. /target/') + ->and($this->volume->fresh()->ignores_compose_driver_options)->toBeFalse(); +}); + +test('a local clone keeps a driver that is not local', function () { + fakeVolumeCloneServer(['Name' => 'clone-source', 'Driver' => 'rclone', 'Options' => ['remote' => 'minio:bucket']]); + + (new VolumeCloneJob('clone-source', 'clone-target', $this->server, $this->server, $this->volume))->handle(); + + expect(volumeCloneCommands())->toContain(escapeshellarg("docker volume create --driver 'rclone' --opt 'remote=minio:bucket' 'clone-target'")); +}); + +test('driver options are escaped for the shell', function () { + fakeVolumeCloneServer([ + 'Name' => 'clone-source', + 'Driver' => 'local', + 'Options' => ['type' => 'cifs', 'o' => "username=a,password=x'; touch /tmp/pwned; echo '\$(id)&&b"], + ]); + + (new VolumeCloneJob('clone-source', 'clone-target', $this->server, $this->server, $this->volume))->handle(); + + $inner = 'docker volume create --opt '.escapeshellarg('type=cifs').' --opt ' + .escapeshellarg("o=username=a,password=x'; touch /tmp/pwned; echo '\$(id)&&b").' '.escapeshellarg('clone-target'); + expect(volumeCloneCommands())->toContain('sh -c '.escapeshellarg($inner)); +}); + +test('a source volume without driver options gives a plain target volume and a name-only declaration', function () { + fakeVolumeCloneServer(['Name' => 'clone-source', 'Driver' => 'local', 'Options' => null]); + + (new VolumeCloneJob('clone-source', 'clone-target', $this->server, $this->server, $this->volume))->handle(); + + expect(volumeCloneCommands())->toContain(escapeshellarg("docker volume create 'clone-target'")) + ->and($this->volume->fresh()->ignores_compose_driver_options)->toBeTrue(); +}); + +test('a missing source volume still creates an empty target volume', function () { + fakeVolumeCloneServer(null); + + (new VolumeCloneJob('clone-source', 'clone-target', $this->server, $this->server, $this->volume))->handle(); + + expect(volumeCloneCommands())->toContain(escapeshellarg("docker volume create 'clone-target'")); +}); + +test('a bind-type source volume is created with its options but its host folder is not copied', function () { + fakeVolumeCloneServer([ + 'Name' => 'clone-source', + 'Driver' => 'local', + 'Options' => ['type' => 'none', 'o' => 'bind', 'device' => '/srv/app-data'], + ]); + + (new VolumeCloneJob('clone-source', 'clone-target', $this->server, $this->server, $this->volume))->handle(); + + $commands = volumeCloneCommands(); + expect($commands)->toContain(escapeshellarg("docker volume create --opt 'type=none' --opt 'o=bind' --opt 'device=/srv/app-data' 'clone-target'")) + ->and($commands)->not->toContain('cp -a') + ->and($commands)->not->toContain('chown -R'); +}); + +test('a remote clone creates the target volume with the source driver options on the target server', function () { + $targetServer = Server::factory()->create([ + 'team_id' => $this->team->id, + 'user' => 'root', + 'private_key_id' => $this->privateKey->id, + ]); + fakeVolumeCloneServer([ + 'Name' => 'clone-source', + 'Driver' => 'local', + 'Options' => ['type' => 'nfs', 'o' => 'addr=10.0.0.5', 'device' => ':/exports/data'], + ]); + + (new VolumeCloneJob('clone-source', 'clone-target', $this->server, $targetServer, $this->volume))->handle(); + + $commands = volumeCloneCommands(); + expect($commands)->toContain(escapeshellarg("docker volume create --opt 'type=nfs' --opt 'o=addr=10.0.0.5' --opt 'device=:/exports/data' 'clone-target'")) + ->and($commands)->toContain('tar xzf /clone/volume-data.tar.gz'); +}); + +test('a remote clone of a bind-type volume does not copy the host folder', function () { + $targetServer = Server::factory()->create([ + 'team_id' => $this->team->id, + 'user' => 'root', + 'private_key_id' => $this->privateKey->id, + ]); + fakeVolumeCloneServer([ + 'Name' => 'clone-source', + 'Driver' => 'local', + 'Options' => ['type' => 'none', 'o' => 'bind,rw', 'device' => '/srv/app-data'], + ]); + + (new VolumeCloneJob('clone-source', 'clone-target', $this->server, $targetServer, $this->volume))->handle(); + + $commands = volumeCloneCommands(); + expect($commands)->toContain(escapeshellarg("docker volume create --opt 'type=none' --opt 'o=bind,rw' --opt 'device=/srv/app-data' 'clone-target'")) + ->and($commands)->not->toContain('tar czf') + ->and($commands)->not->toContain('tar xzf'); +}); + +test('a non-root server runs the volume create as one sudo shell script', function () { + $this->server->update(['user' => 'cooluser']); + Server::flushIdentityMap(); + $server = $this->server->fresh(); + fakeVolumeCloneServer([ + 'Name' => 'clone-source', + 'Driver' => 'local', + 'Options' => ['type' => 'cifs', 'o' => 'password=a&&b|c $(x)'], + ]); + + (new VolumeCloneJob('clone-source', 'clone-target', $server, $server, $this->volume))->handle(); + + $inner = "docker volume create --opt 'type=cifs' --opt 'o=password=a&&b|c \$(x)' 'clone-target'"; + expect(volumeCloneCommands())->toContain('sudo sh -c '.escapeshellarg($inner)) + ->and(volumeCloneCommands())->toContain('sudo docker volume inspect'); +});