fix(backups): close backup directories to other users on non-root servers

app/Jobs/DatabaseBackupJob.php

@@ -598,10 +598,9 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
             Log::info('MongoDB backup URL configured', ['has_url' => filled($url), 'using_env_vars' => blank($this->database->internal_db_url)]);
             $escapedUrl = escapeshellarg($url);
             $escapedContainerName = escapeshellarg($this->container_name);
-            $escapedBackupDir = escapeshellarg($this->backup_dir);
             $escapedBackupLocation = escapeshellarg($this->backup_location);
             if ($databaseWithCollections === 'all') {
-                $commands[] = 'mkdir -p '.$escapedBackupDir;
+                $commands = $this->backupDirectoryCommands();
                 if (str($this->database->image)->startsWith('mongo:4')) {
                     $commands[] = "docker exec {$escapedContainerName} mongodump --uri=$escapedUrl --gzip --archive > {$escapedBackupLocation}";
                 } else {
@@ -615,7 +614,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
                     $databaseName = $databaseWithCollections;
                     $collectionsToExclude = collect();
                 }
-                $commands[] = 'mkdir -p '.$escapedBackupDir;
+                $commands = $this->backupDirectoryCommands();
 
                 // Validate and escape database name to prevent command injection
                 validateShellSafePath($databaseName, 'database name');
@@ -681,7 +680,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
     private function backup_standalone_postgresql(string $database): void
     {
         try {
-            $commands[] = 'mkdir -p '.escapeshellarg($this->backup_dir);
+            $commands = $this->backupDirectoryCommands();
             $backupCommand = 'docker exec';
             if ($this->postgres_password) {
                 $backupCommand .= ' -e PGPASSWORD='.escapeshellarg($this->postgres_password);
@@ -714,7 +713,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
     private function backup_standalone_mysql(string $database): void
     {
         try {
-            $commands[] = 'mkdir -p '.escapeshellarg($this->backup_dir);
+            $commands = $this->backupDirectoryCommands();
             $escapedPassword = escapeshellarg($this->database->mysql_root_password);
             $escapedContainerName = escapeshellarg($this->container_name);
             $escapedBackupLocation = escapeshellarg($this->backup_location);
@@ -741,7 +740,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
     private function backup_standalone_mariadb(string $database): void
     {
         try {
-            $commands[] = 'mkdir -p '.escapeshellarg($this->backup_dir);
+            $commands = $this->backupDirectoryCommands();
             $escapedPassword = escapeshellarg($this->database->mariadb_root_password);
             $escapedContainerName = escapeshellarg($this->container_name);
             $escapedBackupLocation = escapeshellarg($this->backup_location);
@@ -770,12 +769,12 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
         $archiveName = ltrim($this->backup_file, '/');
 
         try {
-            $commands = ClickhouseBackupCommand::make(
+            $commands = [...$this->backupDirectoryCommands(), ...ClickhouseBackupCommand::make(
                 containerName: $this->container_name,
                 database: $database,
                 archiveName: $archiveName,
                 backupDirectory: $this->backup_dir,
-            );
+            )];
 
             $this->backup_output = instant_remote_process($this->writeBackupFileAsRoot($commands), $this->server, true, false, $this->commandTimeout(), disableMultiplexing: true);
             $this->backup_output = trim($this->backup_output);
@@ -797,7 +796,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
             if (! preg_match(StandaloneSqlite::DATABASES_PATTERN, $database)) {
                 throw new \Exception("Invalid database file name: {$database}");
             }
-            $commands[] = 'mkdir -p '.escapeshellarg($this->backup_dir);
+            $commands = $this->backupDirectoryCommands();
             $script = 'f=$(mktemp) && sqlite3 -readonly '.escapeshellarg(StandaloneSqlite::DATA_DIRECTORY.'/'.$database).' \'.timeout 10000\' "VACUUM INTO \'$f\'" && cat "$f"; s=$?; rm -f "$f"; exit $s';
             $dumpCommand = 'docker exec '.escapeshellarg($this->container_name).' sh -c '.escapeshellarg($script);
             $commands[] = $this->buildCompressedDumpCommand($dumpCommand, escapeshellarg($this->backup_location));
@@ -912,6 +911,24 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
         return "{$helperImage}:{$latestVersion}";
     }
 
+    /**
+     * Creates the backup directory. On a server with a non-root SSH user, the directory gets the ownership step
+     * that the sudo parser adds only to unquoted paths: the SSH user owns it (backup downloads read it over SFTP
+     * as that user) and other users cannot read the dumps in it.
+     *
+     * @return array<int, string>
+     */
+    private function backupDirectoryCommands(): array
+    {
+        $backupDirectory = escapeshellarg($this->backup_dir);
+        $commands = ['mkdir -p '.$backupDirectory];
+        if ($this->server->isNonRoot()) {
+            $commands[] = ownershipCommand($backupDirectory, $this->server);
+        }
+
+        return $commands;
+    }
+
     /**
      * The SSH user's shell opens a `>` redirect. For a non-root SSH user, the dump and the redirect into
      * the backup directory must run in one root shell, so each line that writes the backup file becomes

tests/Feature/BackupDirectoryPermissionsTest.php

@@ -0,0 +1,76 @@
+<?php
+
+use App\Jobs\DatabaseBackupJob;
+use App\Models\Environment;
+use App\Models\InstanceSettings;
+use App\Models\PrivateKey;
+use App\Models\Project;
+use App\Models\ScheduledDatabaseBackup;
+use App\Models\Server;
+use App\Models\StandaloneDocker;
+use App\Models\StandaloneMysql;
+use App\Models\Team;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Support\Collection;
+use Illuminate\Support\Facades\Notification;
+use Illuminate\Support\Facades\Process;
+use Illuminate\Support\Facades\Storage;
+
+uses(RefreshDatabase::class);
+
+beforeEach(function () {
+    Server::flushIdentityMap();
+    Storage::fake('ssh-keys');
+    Storage::fake('ssh-mux');
+    config(['constants.ssh.mux_enabled' => false]);
+    InstanceSettings::forceCreate(['id' => 0]);
+    Notification::fake();
+});
+
+/**
+ * Runs a MySQL backup on a server with the given SSH user and returns the remote script of the dump command.
+ */
+function backupDirectoryDumpScript(string $sshUser): string
+{
+    $team = Team::factory()->create(['name' => 'Acme']);
+    $server = Server::factory()->create(['team_id' => $team->id, 'user' => $sshUser]);
+    $server->update(['private_key_id' => PrivateKey::factory()->create(['team_id' => $team->id])->id]);
+    $server->settings()->update(['is_reachable' => true, 'is_usable' => true]);
+    $project = Project::factory()->create(['team_id' => $team->id]);
+    $environment = Environment::factory()->create(['project_id' => $project->id]);
+    $database = create_standalone_mysql($environment->id, StandaloneDocker::where('server_id', $server->id)->firstOrFail(), [
+        'mysql_database' => 'app',
+        'status' => 'running:healthy',
+    ]);
+    $backup = ScheduledDatabaseBackup::create([
+        'frequency' => '0 0 * * *',
+        'save_s3' => false,
+        'database_type' => StandaloneMysql::class,
+        'database_id' => $database->id,
+        'team_id' => $team->id,
+        'databases_to_backup' => 'app',
+    ]);
+    $commands = new Collection;
+    Process::fake(function ($process) use ($commands) {
+        $commands->push($process->command);
+
+        return Process::result(output: str_contains($process->command, 'du -b') ? '100' : '');
+    });
+
+    (new DatabaseBackupJob($backup))->handle();
+
+    return $commands->first(fn (string $command) => str_contains($command, 'mysqldump'));
+}
+
+it('gives the backup directory to the ssh user and closes it to other users on a non-root server', function () {
+    $script = backupDirectoryDumpScript('ubuntu');
+
+    expect($script)->toMatch("~^sudo mkdir -p '(/data/coolify/backups/databases/acme-\\d+/[^']+)'\\nsudo find '\\1' -user root -exec chown [^{]*ubuntu:ubuntu \\{\\} \\+ && sudo chmod o-rwx '\\1'\\n.*mysqldump~m");
+});
+
+it('leaves the backup directory command unchanged on a root server', function () {
+    $script = backupDirectoryDumpScript('root');
+
+    expect($script)->toMatch("~^mkdir -p '/data/coolify/backups/databases/acme-\\d+/[^']+'\\n.*mysqldump~m")
+        ->and($script)->not->toContain('chmod o-rwx');
+});