diff --git a/app/Livewire/Project/Database/ImportForm.php b/app/Livewire/Project/Database/ImportForm.php index 3b72383a5..ccc7b347d 100644 --- a/app/Livewire/Project/Database/ImportForm.php +++ b/app/Livewire/Project/Database/ImportForm.php @@ -685,13 +685,21 @@ EOD; $containerTmpPath = "/tmp/restore_{$this->resourceUuid}-".basename($cleanPath); $scriptPath = "/tmp/restore_{$this->resourceUuid}.sh"; + $escapedServerTmpPath = escapeshellarg($serverTmpPath); + $escapedContainerTmpPath = escapeshellarg($containerTmpPath); + $escapedScriptPath = escapeshellarg($scriptPath); + $escapedHelperContainerPath = escapeshellarg("{$containerName}:{$helperTmpPath}"); + $escapedDatabaseContainerTmpPath = escapeshellarg("{$this->container}:{$containerTmpPath}"); + $escapedDatabaseContainerScriptPath = escapeshellarg("{$this->container}:{$scriptPath}"); + $restoreAndCleanupCommand = escapeshellarg("{$escapedScriptPath} && rm -f {$escapedContainerTmpPath} {$escapedScriptPath}"); + // Prepare all commands in sequence $commands = []; // 1. Clean up any existing helper container and temp files from previous runs $commands[] = "docker rm -f {$containerName} 2>/dev/null || true"; - $commands[] = "rm -f {$serverTmpPath} 2>/dev/null || true"; - $commands[] = "docker exec {$this->container} rm -f {$containerTmpPath} {$scriptPath} 2>/dev/null || true"; + $commands[] = "rm -f {$escapedServerTmpPath} 2>/dev/null || true"; + $commands[] = "docker exec {$this->container} rm -f {$escapedContainerTmpPath} {$escapedScriptPath} 2>/dev/null || true"; // 2. Start helper container on the database network $commands[] = "docker run -d --network {$destinationNetwork} --name {$containerName} {$fullImageName} sleep 3600"; @@ -703,8 +711,6 @@ EOD; $commands[] = "docker exec {$containerName} mc alias set s3temp {$escapedEndpoint} {$escapedKey} {$escapedSecret}"; // 4. Check file exists in S3 (bucket and path already validated above) - $escapedBucket = escapeshellarg($bucket); - $escapedCleanPath = escapeshellarg($cleanPath); $escapedS3Source = escapeshellarg("s3temp/{$bucket}/{$cleanPath}"); $commands[] = "docker exec {$containerName} mc stat {$escapedS3Source}"; @@ -713,23 +719,23 @@ EOD; $commands[] = "docker exec {$containerName} mc cp {$escapedS3Source} {$escapedHelperTmpPath}"; // 6. Copy from helper to server, then immediately to database container - $commands[] = "docker cp {$containerName}:{$helperTmpPath} {$serverTmpPath}"; - $commands[] = "docker cp {$serverTmpPath} {$this->container}:{$containerTmpPath}"; + $commands[] = "docker cp {$escapedHelperContainerPath} {$escapedServerTmpPath}"; + $commands[] = "docker cp {$escapedServerTmpPath} {$escapedDatabaseContainerTmpPath}"; // 7. Cleanup helper container and server temp file immediately (no longer needed) $commands[] = "docker rm -f {$containerName} 2>/dev/null || true"; - $commands[] = "rm -f {$serverTmpPath} 2>/dev/null || true"; + $commands[] = "rm -f {$escapedServerTmpPath} 2>/dev/null || true"; // 8. Build and execute restore command inside database container $restoreCommand = $this->buildRestoreCommand($containerTmpPath); $restoreCommandBase64 = base64_encode($restoreCommand); - $commands[] = "echo \"{$restoreCommandBase64}\" | base64 -d > {$scriptPath}"; - $commands[] = "chmod +x {$scriptPath}"; - $commands[] = "docker cp {$scriptPath} {$this->container}:{$scriptPath}"; + $commands[] = "echo \"{$restoreCommandBase64}\" | base64 -d > {$escapedScriptPath}"; + $commands[] = "chmod +x {$escapedScriptPath}"; + $commands[] = "docker cp {$escapedScriptPath} {$escapedDatabaseContainerScriptPath}"; // 9. Execute restore and cleanup temp files immediately after completion - $commands[] = "docker exec {$this->container} sh -c '{$scriptPath} && rm -f {$containerTmpPath} {$scriptPath}'"; + $commands[] = "docker exec {$this->container} sh -c {$restoreAndCleanupCommand}"; $commands[] = "docker exec {$this->container} sh -c 'echo \"Import finished with exit code $?\"'"; // Execute all commands with cleanup event (as safety net for edge cases) @@ -761,6 +767,7 @@ EOD; public function buildRestoreCommand(string $tmpPath): string { + $escapedTmpPath = escapeshellarg($tmpPath); $morphClass = $this->resource->getMorphClass(); // Handle ServiceDatabase by checking the database type @@ -782,32 +789,32 @@ EOD; case 'mariadb': $restoreCommand = $this->mariadbRestoreCommand; if ($this->dumpAll) { - $restoreCommand .= " && (gunzip -cf {$tmpPath} 2>/dev/null || cat {$tmpPath}) | mariadb -u root -p\$MARIADB_ROOT_PASSWORD \${MARIADB_DATABASE:-default}"; + $restoreCommand .= " && (gunzip -cf {$escapedTmpPath} 2>/dev/null || cat {$escapedTmpPath}) | mariadb -u root -p\$MARIADB_ROOT_PASSWORD \${MARIADB_DATABASE:-default}"; } else { - $restoreCommand .= " < {$tmpPath}"; + $restoreCommand .= " < {$escapedTmpPath}"; } break; case StandaloneMysql::class: case 'mysql': $restoreCommand = $this->mysqlRestoreCommand; if ($this->dumpAll) { - $restoreCommand .= " && (gunzip -cf {$tmpPath} 2>/dev/null || cat {$tmpPath}) | mysql -u root -p\$MYSQL_ROOT_PASSWORD \${MYSQL_DATABASE:-default}"; + $restoreCommand .= " && (gunzip -cf {$escapedTmpPath} 2>/dev/null || cat {$escapedTmpPath}) | mysql -u root -p\$MYSQL_ROOT_PASSWORD \${MYSQL_DATABASE:-default}"; } else { - $restoreCommand .= " < {$tmpPath}"; + $restoreCommand .= " < {$escapedTmpPath}"; } break; case StandalonePostgresql::class: case 'postgresql': $restoreCommand = $this->postgresqlRestoreCommand; if ($this->dumpAll) { - $restoreCommand .= " && (gunzip -cf {$tmpPath} 2>/dev/null || cat {$tmpPath}) | psql -U \${POSTGRES_USER} -d \${POSTGRES_DB:-\${POSTGRES_USER:-postgres}}"; + $restoreCommand .= " && (gunzip -cf {$escapedTmpPath} 2>/dev/null || cat {$escapedTmpPath}) | psql -U \${POSTGRES_USER} -d \${POSTGRES_DB:-\${POSTGRES_USER:-postgres}}"; } else { - $restoreCommand .= " {$tmpPath}"; + $restoreCommand .= " {$escapedTmpPath}"; } break; case StandaloneMongodb::class: case 'mongodb': - $restoreCommand = $this->mongodbRestoreCommand."{$tmpPath}"; + $restoreCommand = $this->mongodbRestoreCommand.$escapedTmpPath; break; default: $restoreCommand = ''; diff --git a/tests/Feature/DatabaseImportCommandInjectionTest.php b/tests/Feature/DatabaseImportCommandInjectionTest.php index f7b1bbbed..114d19884 100644 --- a/tests/Feature/DatabaseImportCommandInjectionTest.php +++ b/tests/Feature/DatabaseImportCommandInjectionTest.php @@ -1,7 +1,8 @@ getAttributes(\Livewire\Attributes\Locked::class); + $property = new ReflectionProperty(ImportForm::class, 'container'); + $attributes = $property->getAttributes(Locked::class); expect($attributes)->not->toBeEmpty(); }); test('serverId property has Locked attribute', function () { - $property = new ReflectionProperty(Import::class, 'serverId'); - $attributes = $property->getAttributes(\Livewire\Attributes\Locked::class); + $property = new ReflectionProperty(ImportForm::class, 'serverId'); + $attributes = $property->getAttributes(Locked::class); expect($attributes)->not->toBeEmpty(); }); test('resourceId property has Locked attribute', function () { - $property = new ReflectionProperty(Import::class, 'resourceId'); - $attributes = $property->getAttributes(\Livewire\Attributes\Locked::class); + $property = new ReflectionProperty(ImportForm::class, 'resourceId'); + $attributes = $property->getAttributes(Locked::class); expect($attributes)->not->toBeEmpty(); }); test('resourceType property has Locked attribute', function () { - $property = new ReflectionProperty(Import::class, 'resourceType'); - $attributes = $property->getAttributes(\Livewire\Attributes\Locked::class); + $property = new ReflectionProperty(ImportForm::class, 'resourceType'); + $attributes = $property->getAttributes(Locked::class); expect($attributes)->not->toBeEmpty(); }); test('resourceUuid property has Locked attribute', function () { - $property = new ReflectionProperty(Import::class, 'resourceUuid'); - $attributes = $property->getAttributes(\Livewire\Attributes\Locked::class); + $property = new ReflectionProperty(ImportForm::class, 'resourceUuid'); + $attributes = $property->getAttributes(Locked::class); expect($attributes)->not->toBeEmpty(); }); test('resourceDbType property has Locked attribute', function () { - $property = new ReflectionProperty(Import::class, 'resourceDbType'); - $attributes = $property->getAttributes(\Livewire\Attributes\Locked::class); + $property = new ReflectionProperty(ImportForm::class, 'resourceDbType'); + $attributes = $property->getAttributes(Locked::class); expect($attributes)->not->toBeEmpty(); }); @@ -89,7 +90,7 @@ describe('locked properties', function () { describe('server method uses team scoping', function () { test('server computed property calls ownedByCurrentTeam', function () { - $method = new ReflectionMethod(Import::class, 'server'); + $method = new ReflectionMethod(ImportForm::class, 'server'); // Extract the server method body $startLine = $method->getStartLine(); @@ -102,9 +103,9 @@ describe('server method uses team scoping', function () { }); }); -describe('Import component uses shared ValidationPatterns', function () { +describe('ImportForm component uses shared ValidationPatterns', function () { test('runImport references ValidationPatterns for container validation', function () { - $method = new ReflectionMethod(Import::class, 'runImport'); + $method = new ReflectionMethod(ImportForm::class, 'runImport'); $startLine = $method->getStartLine(); $endLine = $method->getEndLine(); $lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1); @@ -114,7 +115,7 @@ describe('Import component uses shared ValidationPatterns', function () { }); test('restoreFromS3 references ValidationPatterns for container validation', function () { - $method = new ReflectionMethod(Import::class, 'restoreFromS3'); + $method = new ReflectionMethod(ImportForm::class, 'restoreFromS3'); $startLine = $method->getStartLine(); $endLine = $method->getEndLine(); $lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1); diff --git a/tests/Unit/Project/Database/ImportCheckFileButtonTest.php b/tests/Unit/Project/Database/ImportCheckFileButtonTest.php index a305160c0..8d8d29de8 100644 --- a/tests/Unit/Project/Database/ImportCheckFileButtonTest.php +++ b/tests/Unit/Project/Database/ImportCheckFileButtonTest.php @@ -1,15 +1,11 @@ customLocation = ''; - $mockServer = Mockery::mock(Server::class); - $component->server = $mockServer; - // No server commands should be executed when customLocation is empty $component->checkFile(); @@ -17,19 +13,16 @@ test('checkFile does nothing when customLocation is empty', function () { }); test('checkFile validates file exists on server when customLocation is filled', function () { - $component = new Import; + $component = new ImportForm; $component->customLocation = '/tmp/backup.sql'; - $mockServer = Mockery::mock(Server::class); - $component->server = $mockServer; - // This test verifies the logic flows when customLocation has a value // The actual remote process execution is tested elsewhere expect($component->customLocation)->toBe('/tmp/backup.sql'); }); test('customLocation can be cleared to allow uploaded file to be used', function () { - $component = new Import; + $component = new ImportForm; $component->customLocation = '/tmp/backup.sql'; // Simulate clearing the customLocation (as happens when file is uploaded) @@ -39,7 +32,7 @@ test('customLocation can be cleared to allow uploaded file to be used', function }); test('validateBucketName accepts valid bucket names', function () { - $component = new Import; + $component = new ImportForm; $method = new ReflectionMethod($component, 'validateBucketName'); // Valid bucket names @@ -51,7 +44,7 @@ test('validateBucketName accepts valid bucket names', function () { }); test('validateBucketName rejects invalid bucket names', function () { - $component = new Import; + $component = new ImportForm; $method = new ReflectionMethod($component, 'validateBucketName'); // Invalid bucket names (command injection attempts) @@ -65,7 +58,7 @@ test('validateBucketName rejects invalid bucket names', function () { }); test('validateS3Path accepts valid S3 paths', function () { - $component = new Import; + $component = new ImportForm; $method = new ReflectionMethod($component, 'validateS3Path'); // Valid S3 paths @@ -77,7 +70,7 @@ test('validateS3Path accepts valid S3 paths', function () { }); test('validateS3Path rejects invalid S3 paths', function () { - $component = new Import; + $component = new ImportForm; $method = new ReflectionMethod($component, 'validateS3Path'); // Invalid S3 paths (command injection attempts) @@ -97,7 +90,7 @@ test('validateS3Path rejects invalid S3 paths', function () { }); test('validateServerPath accepts valid server paths', function () { - $component = new Import; + $component = new ImportForm; $method = new ReflectionMethod($component, 'validateServerPath'); // Valid server paths (must be absolute) @@ -108,7 +101,7 @@ test('validateServerPath accepts valid server paths', function () { }); test('validateServerPath rejects invalid server paths', function () { - $component = new Import; + $component = new ImportForm; $method = new ReflectionMethod($component, 'validateServerPath'); // Invalid server paths diff --git a/tests/Unit/S3RestoreSecurityTest.php b/tests/Unit/S3RestoreSecurityTest.php index c224ec48c..51c374af5 100644 --- a/tests/Unit/S3RestoreSecurityTest.php +++ b/tests/Unit/S3RestoreSecurityTest.php @@ -1,8 +1,14 @@ toContain("'my'\\''secret'\\''key'"); }); + +it('quotes restore command temp paths with spaces', function (string $morphClass) { + $component = new class extends ImportForm + { + public string $morphClass; + + public function __get($property) + { + if ($property === 'resource') { + return new class($this->morphClass) + { + public function __construct(private readonly string $morphClass) {} + + public function getMorphClass(): string + { + return $this->morphClass; + } + }; + } + + return parent::__get($property); + } + }; + $component->morphClass = $morphClass; + + $tmpPath = '/tmp/restore_test-may 2026.sql.gz'; + $restoreCommand = $component->buildRestoreCommand($tmpPath); + + expect($restoreCommand) + ->toContain(escapeshellarg($tmpPath)) + ->not->toContain(" {$tmpPath}"); +})->with([ + 'mariadb' => StandaloneMariadb::class, + 'mysql' => StandaloneMysql::class, + 'postgresql' => StandalonePostgresql::class, + 'mongodb' => StandaloneMongodb::class, +]); + +it('quotes dump all restore command temp paths with spaces', function (string $morphClass) { + $component = new class extends ImportForm + { + public string $morphClass; + + public function __get($property) + { + if ($property === 'resource') { + return new class($this->morphClass) + { + public function __construct(private readonly string $morphClass) {} + + public function getMorphClass(): string + { + return $this->morphClass; + } + }; + } + + return parent::__get($property); + } + }; + $component->morphClass = $morphClass; + $component->dumpAll = true; + + $tmpPath = '/tmp/restore_test-may 2026.sql.gz'; + $escapedTmpPath = escapeshellarg($tmpPath); + $restoreCommand = $component->buildRestoreCommand($tmpPath); + + expect($restoreCommand) + ->toContain("gunzip -cf {$escapedTmpPath}") + ->toContain("cat {$escapedTmpPath}") + ->not->toContain("gunzip -cf {$tmpPath}") + ->not->toContain("cat {$tmpPath}"); +})->with([ + 'mariadb' => StandaloneMariadb::class, + 'mysql' => StandaloneMysql::class, + 'postgresql' => StandalonePostgresql::class, +]);