fix(database-import): improve MySQL and MariaDB restore handling (#12352)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

app/Support/DatabaseImport/DatabaseImportCommandBuilder.php

@@ -54,6 +54,17 @@ tolower($0) ~ /^[ \t]*create[ \t]+(database|schema)[ \t]/ { if (is_system(databa
 !skip { print }
 AWK;
 
+    /**
+     * mysql and mariadb client commands that run a shell command, write or read a file:
+     * system, tee, pager, edit and source, as a long command at the start of a line or as
+     * a short command (\!, \T, \P, \e, \.) anywhere. An odd backslash run starts a short
+     * command; mysqldump writes a backslash in data as "\\" and escapes only 0, n, r, Z and
+     * quotes, so its rows do not match.
+     */
+    private const MYSQL_CLIENT_COMMAND_PATTERN = '(^|[^\\\\])(\\\\\\\\)*\\\\[!TPe.]';
+
+    private const MYSQL_NAMED_CLIENT_COMMAND_PATTERN = '^[[:space:]]*(system|tee|pager|edit|source)([[:space:]]|;|$)';
+
     /**
      * Prints a PostgreSQL SQL dump without the table data of its COPY ... FROM stdin blocks.
      */
@@ -317,24 +328,42 @@ SH;
      * MySQL and MariaDB restore SQL dumps. A tar backup must wrap exactly one dump.
      * The all-databases mode checks the backup before it drops any database and skips
      * the system databases unless their users and privileges are restored as well.
+     * The client runs the dump with its client commands disabled: --sandbox on MariaDB
+     * (10.5.25, 10.6.18, 10.11.8, 11.4.2 and later), --commands=FALSE on MySQL (8.0.43, 8.4.6,
+     * 9.4.0 and later). Older clients do not know these options, so a dump with a client
+     * command that runs a shell command or uses a file is refused on every client.
      */
     private function mysql(string $binary, string $prefix, bool $dumpAll, bool $restoreMysqlUsers = false): string
     {
-        $preflight = <<<'SH'
+        $commandPattern = escapeshellarg(self::MYSQL_CLIENT_COMMAND_PATTERN);
+        $namedCommandPattern = escapeshellarg(self::MYSQL_NAMED_CLIENT_COMMAND_PATTERN);
+        $preflight = <<<SH
 if is_tar; then
   extract_tar
   use_single_tar_member || fail 'A tar backup must contain exactly one SQL dump. Nothing was changed.'
 fi
 is_text || fail 'Unsupported backup format. Use an SQL dump. Nothing was changed.'
+if stream | LC_ALL=C grep -Eq {$commandPattern} || stream | LC_ALL=C grep -Eiq {$namedCommandPattern}; then
+  fail 'Blocked restore: the backup contains a {$binary} client command (system, tee, pager, edit or source). If you trust this dump, restore it from the database container terminal. Nothing was changed.'
+fi
+
+SH;
+        $clientSafety = <<<SH
+client_safety=
+if {$binary} --help 2>/dev/null | grep -q -e '--sandbox'; then
+  client_safety=--sandbox
+elif {$binary} --help 2>/dev/null | grep -q -e '--commands'; then
+  client_safety=--commands=FALSE
+fi
 
 SH;
 
         if (! $dumpAll) {
-            return $preflight.<<<SH
+            return $preflight.$clientSafety.<<<SH
 if [ "$(stream | grep -c '^USE `')" -gt 1 ]; then
   fail 'This backup contains more than one database. Select "Backup contains all databases" to restore it. Nothing was changed.'
 fi
-stream | {$binary} -u \${$prefix}_USER -p\${$prefix}_PASSWORD \${$prefix}_DATABASE
+stream | {$binary} \$client_safety -u \${$prefix}_USER -p\${$prefix}_PASSWORD \${$prefix}_DATABASE
 SH;
         }
 
@@ -350,11 +379,11 @@ SH;
             $source = '{ stream | awk '.escapeshellarg(self::MYSQL_SYSTEM_DATABASE_FILTER)." || echo 'Coolify could not filter the system databases of the backup;'; }";
         }
 
-        return $preflight.<<<SH
+        return $preflight.$clientSafety.<<<SH
 for pid in \$({$root} -N -e "SELECT id FROM information_schema.processlist WHERE user != 'root';"); do {$root} -e "KILL \$pid" 2>/dev/null || true; done
 {$root} -N -e "SELECT CONCAT('DROP DATABASE IF EXISTS \\`',schema_name,'\\`;') FROM information_schema.schemata WHERE schema_name NOT IN ('information_schema','mysql','performance_schema','sys');" | {$root} || exit 1
 {$root} -e "CREATE DATABASE IF NOT EXISTS \\`{$database}\\`;" || exit 1
-{$source} | {$root} {$database}
+{$source} | {$binary} \$client_safety -u root -p\${{$prefix}_ROOT_PASSWORD} {$database}
 SH;
     }
 

tests/Unit/DatabaseImport/DatabaseImportCommandBuilderTest.php

@@ -31,13 +31,13 @@ test('builds database-specific restore commands', function (string $class, ?stri
     expect($command)->toContain($needle)->toContain("'/tmp/restore file'");
 })->with([
     'postgresql' => [StandalonePostgresql::class, null, 'pg_restore'],
-    'mysql' => [StandaloneMysql::class, null, 'mysql -u $MYSQL_USER'],
-    'mariadb' => [StandaloneMariadb::class, null, 'mariadb -u $MARIADB_USER'],
+    'mysql' => [StandaloneMysql::class, null, 'mysql $client_safety -u $MYSQL_USER'],
+    'mariadb' => [StandaloneMariadb::class, null, 'mariadb $client_safety -u $MARIADB_USER'],
     'mongodb' => [StandaloneMongodb::class, null, 'mongorestore'],
     'sqlite' => [StandaloneSqlite::class, null, '.restore'],
     'service postgres' => [ServiceDatabase::class, 'postgresql', 'pg_restore'],
-    'service mysql' => [ServiceDatabase::class, 'mysql', 'mysql -u $MYSQL_USER'],
-    'service mariadb' => [ServiceDatabase::class, 'mariadb', 'mariadb -u $MARIADB_USER'],
+    'service mysql' => [ServiceDatabase::class, 'mysql', 'mysql $client_safety -u $MYSQL_USER'],
+    'service mariadb' => [ServiceDatabase::class, 'mariadb', 'mariadb $client_safety -u $MARIADB_USER'],
     'service mongo' => [ServiceDatabase::class, 'mongodb', 'mongorestore'],
 ]);
 
@@ -51,10 +51,10 @@ test('decompresses gzip backups for single-database mysql and mariadb restores',
         ->toContain('is_gzip() { [ "$(head -c 2 "$backup" | od -An -tx1 | tr -d \' \n\')" = 1f8b ]; }')
         ->toEndWith("stream | {$client}");
 })->with([
-    'mysql' => [StandaloneMysql::class, null, 'mysql -u $MYSQL_USER -p$MYSQL_PASSWORD $MYSQL_DATABASE'],
-    'mariadb' => [StandaloneMariadb::class, null, 'mariadb -u $MARIADB_USER -p$MARIADB_PASSWORD $MARIADB_DATABASE'],
-    'service mysql' => [ServiceDatabase::class, 'mysql', 'mysql -u $MYSQL_USER -p$MYSQL_PASSWORD $MYSQL_DATABASE'],
-    'service mariadb' => [ServiceDatabase::class, 'mariadb', 'mariadb -u $MARIADB_USER -p$MARIADB_PASSWORD $MARIADB_DATABASE'],
+    'mysql' => [StandaloneMysql::class, null, 'mysql $client_safety -u $MYSQL_USER -p$MYSQL_PASSWORD $MYSQL_DATABASE'],
+    'mariadb' => [StandaloneMariadb::class, null, 'mariadb $client_safety -u $MARIADB_USER -p$MARIADB_PASSWORD $MARIADB_DATABASE'],
+    'service mysql' => [ServiceDatabase::class, 'mysql', 'mysql $client_safety -u $MYSQL_USER -p$MYSQL_PASSWORD $MYSQL_DATABASE'],
+    'service mariadb' => [ServiceDatabase::class, 'mariadb', 'mariadb $client_safety -u $MARIADB_USER -p$MARIADB_PASSWORD $MARIADB_DATABASE'],
 ]);
 
 test('builds dump-all commands and postgres safety scan', function () {
@@ -158,7 +158,7 @@ test('skips the system databases of MySQL and MariaDB all-databases backups by d
     expect($command)->toContain('command -v awk')
         ->toMatch('/stream \| \{?\s*awk /')
         ->toContain('"mysql"')->toContain('"sys"')->toContain('"performance_schema"')->toContain('"information_schema"')
-        ->toContain("| {$binary} -u root");
+        ->toContain("| {$binary} \$client_safety -u root");
 })->with([
     'mysql' => [StandaloneMysql::class, 'mysql'],
     'mariadb' => [StandaloneMariadb::class, 'mariadb'],
@@ -168,7 +168,7 @@ test('restores the system databases of MySQL and MariaDB all-databases backups w
     $command = (new DatabaseImportCommandBuilder)->buildRestoreCommand(importResource($class), '/tmp/backup.sql', true, restoreMysqlUsers: true);
 
     expect($command)->not->toContain('awk')
-        ->toContain("stream | {$binary} -u root");
+        ->toContain("stream | {$binary} \$client_safety -u root");
 })->with([
     'mysql' => [StandaloneMysql::class, 'mysql'],
     'mariadb' => [StandaloneMariadb::class, 'mariadb'],

tests/Unit/DatabaseImport/DatabaseImportRestoreScriptTest.php

@@ -250,6 +250,7 @@ function restoreScriptRun(string $engine, string $contents, bool $dumpAll = fals
             $escapedStdin = escapeshellarg($dir.'/stdin');
             file_put_contents($dir.'/bin/'.$client, <<<SH
 #!/bin/sh
+if [ "\$*" = '--help' ]; then printf '%s\\n' "\${STUB_HELP:-}"; exit 0; fi
 header=''
 call=\$(wc -l < {$escapedLog} | tr -d ' ')
 if [ ! -t 0 ]; then
@@ -735,6 +736,47 @@ test('rejects unsupported MySQL and MariaDB backups before calling any client',
     'dump of two databases without dump-all' => ['mysql-two-databases', false, 'This backup contains more than one database.'],
 ]);
 
+test('restores MySQL and MariaDB dumps with client commands disabled when the client supports it', function (string $engine, string $help, string $flag, bool $dumpAll) {
+    $run = restoreScriptRun($engine, restoreScriptFixture('mysql-sql'), $dumpAll, env: ['STUB_HELP' => $help]);
+
+    expect($run['exit'])->toBe(0, $run['stderr'])
+        ->and(end($run['calls'])['args'])->toStartWith($flag.' -u ')
+        ->and(end($run['calls'])['stdin'])->toBe(restoreScriptFixture('mysql-sql'));
+})->with(['mysql', 'mariadb'])->with([
+    'MariaDB client' => ['  --sandbox           Disallow commands that access the file system', '--sandbox'],
+    'MySQL client' => ['  --commands          Whether to enable or disable processing of local mysql commands.', '--commands=FALSE'],
+])->with(['single database' => false, 'all databases' => true]);
+
+test('refuses MySQL and MariaDB dumps with client commands before calling any client', function (string $engine, string $command, bool $dumpAll) {
+    $dump = "-- MySQL dump 10.13\n\nCREATE TABLE `items` (`id` int);\n{$command}\nINSERT INTO `items` VALUES (1);\n";
+
+    foreach ([$dump, gzencode($dump)] as $contents) {
+        $run = restoreScriptRun($engine, $contents, $dumpAll);
+
+        expect($run['exit'])->toBe(1)
+            ->and($run['stderr'])->toContain('client command')->toContain('Nothing was changed.')
+            ->and($run['calls'])->toBe([]);
+    }
+})->with(['mysql', 'mariadb'])->with([
+    'system' => 'system id',
+    'indented SYSTEM' => '  SYSTEM id;',
+    '\\!' => '\\! id',
+    '\\! after SQL' => 'SELECT 1; \\! id',
+    'tee' => 'tee /etc/cron.d/x',
+    '\\T' => '\\T /etc/cron.d/x',
+    'pager' => 'pager sh',
+    'source' => 'source /etc/shadow',
+    '\\.' => '\\. /etc/shadow',
+])->with(['single database' => false, 'all databases' => true]);
+
+test('restores MySQL and MariaDB dumps whose data only looks like client commands', function (string $engine) {
+    $dump = "-- MySQL dump 10.13\n/*M!999999\\- enable the sandbox mode */\nCREATE TABLE `items` (`t` text, `system` int);\nINSERT INTO `items` VALUES ('a\\\\! b\\\\\\\\T c\\'s \\\\. \\r\\n',1),('system id',2);\n";
+    $run = restoreScriptRun($engine, $dump);
+
+    expect($run['exit'])->toBe(0, $run['stderr'])
+        ->and(end($run['calls'])['stdin'])->toBe($dump);
+})->with(['mysql', 'mariadb']);
+
 test('restores mongodump archives', function (string $fixture, bool $replaceExisting, bool $gzip) {
     $run = restoreScriptRun('mongodb', restoreScriptFixture($fixture), false, $replaceExisting);
 

tests/Unit/Livewire/Database/S3RestoreTest.php

@@ -162,7 +162,7 @@ test('buildRestoreCommand handles MySQL without dumpAll', function () {
     $result = $component->buildRestoreCommand('/tmp/test.dump');
 
     expect($result)->toStartWith("backup='/tmp/test.dump'");
-    expect($result)->toContain('stream | mysql -u $MYSQL_USER -p$MYSQL_PASSWORD $MYSQL_DATABASE');
+    expect($result)->toContain('stream | mysql $client_safety -u $MYSQL_USER -p$MYSQL_PASSWORD $MYSQL_DATABASE');
     expect($result)->not->toContain("< '/tmp/test.dump'");
 });
 
@@ -173,7 +173,7 @@ test('buildRestoreCommand handles MariaDB without dumpAll', function () {
     $result = $component->buildRestoreCommand('/tmp/test.dump');
 
     expect($result)->toStartWith("backup='/tmp/test.dump'");
-    expect($result)->toContain('stream | mariadb -u $MARIADB_USER -p$MARIADB_PASSWORD $MARIADB_DATABASE');
+    expect($result)->toContain('stream | mariadb $client_safety -u $MARIADB_USER -p$MARIADB_PASSWORD $MARIADB_DATABASE');
     expect($result)->not->toContain("< '/tmp/test.dump'");
 });