diff --git a/app/Events/RestoreJobFinished.php b/app/Events/RestoreJobFinished.php index cc4be8029..8690e01f6 100644 --- a/app/Events/RestoreJobFinished.php +++ b/app/Events/RestoreJobFinished.php @@ -22,11 +22,11 @@ public function __construct($data) $commands = []; if (isSafeTmpPath($scriptPath)) { - $commands[] = "docker exec {$container} sh -c 'rm ".escapeshellarg($scriptPath)." 2>/dev/null || true'"; + $commands[] = 'docker exec '.escapeshellarg($container)." sh -c 'rm ".escapeshellarg($scriptPath)." 2>/dev/null || true'"; } if (isSafeTmpPath($tmpPath)) { - $commands[] = "docker exec {$container} sh -c 'rm ".escapeshellarg($tmpPath)." 2>/dev/null || true'"; + $commands[] = 'docker exec '.escapeshellarg($container)." sh -c 'rm ".escapeshellarg($tmpPath)." 2>/dev/null || true'"; } if (! empty($commands)) { diff --git a/app/Livewire/Project/Database/Import.php b/app/Livewire/Project/Database/Import.php index 01ddb7f5d..fd191e587 100644 --- a/app/Livewire/Project/Database/Import.php +++ b/app/Livewire/Project/Database/Import.php @@ -13,6 +13,92 @@ class Import extends Component { use AuthorizesRequests; + /** + * Validate that a string is safe for use as an S3 bucket name. + * Allows alphanumerics, dots, dashes, and underscores. + */ + private function validateBucketName(string $bucket): bool + { + return preg_match('/^[a-zA-Z0-9.\-_]+$/', $bucket) === 1; + } + + /** + * Validate that a string is safe for use as an S3 path. + * Allows alphanumerics, dots, dashes, underscores, slashes, and common file characters. + */ + private function validateS3Path(string $path): bool + { + // Must not be empty + if (empty($path)) { + return false; + } + + // Must not contain dangerous shell metacharacters or command injection patterns + $dangerousPatterns = [ + '..', // Directory traversal + '$(', // Command substitution + '`', // Backtick command substitution + '|', // Pipe + ';', // Command separator + '&', // Background/AND + '>', // Redirect + '<', // Redirect + "\n", // Newline + "\r", // Carriage return + "\0", // Null byte + "'", // Single quote + '"', // Double quote + '\\', // Backslash + ]; + + foreach ($dangerousPatterns as $pattern) { + if (str_contains($path, $pattern)) { + return false; + } + } + + // Allow alphanumerics, dots, dashes, underscores, slashes, spaces, plus, equals, at + return preg_match('/^[a-zA-Z0-9.\-_\/\s+@=]+$/', $path) === 1; + } + + /** + * Validate that a string is safe for use as a file path on the server. + */ + private function validateServerPath(string $path): bool + { + // Must be an absolute path + if (! str_starts_with($path, '/')) { + return false; + } + + // Must not contain dangerous shell metacharacters or command injection patterns + $dangerousPatterns = [ + '..', // Directory traversal + '$(', // Command substitution + '`', // Backtick command substitution + '|', // Pipe + ';', // Command separator + '&', // Background/AND + '>', // Redirect + '<', // Redirect + "\n", // Newline + "\r", // Carriage return + "\0", // Null byte + "'", // Single quote + '"', // Double quote + '\\', // Backslash + ]; + + foreach ($dangerousPatterns as $pattern) { + if (str_contains($path, $pattern)) { + return false; + } + } + + // Allow alphanumerics, dots, dashes, underscores, slashes, and spaces + return preg_match('/^[a-zA-Z0-9.\-_\/\s]+$/', $path) === 1; + } + public bool $unsupported = false; public $resource; @@ -160,8 +246,16 @@ public function getContainers() public function checkFile() { if (filled($this->customLocation)) { + // Validate the custom location to prevent command injection + if (! $this->validateServerPath($this->customLocation)) { + $this->dispatch('error', 'Invalid file path. Path must be absolute and contain only safe characters (alphanumerics, dots, dashes, underscores, slashes).'); + + return; + } + try { - $result = instant_remote_process(["ls -l {$this->customLocation}"], $this->server, throwError: false); + $escapedPath = escapeshellarg($this->customLocation); + $result = instant_remote_process(["ls -l {$escapedPath}"], $this->server, throwError: false); if (blank($result)) { $this->dispatch('error', 'The file does not exist or has been deleted.'); @@ -197,8 +291,15 @@ public function runImport() Storage::delete($backupFileName); $this->importCommands[] = "docker cp {$tmpPath} {$this->container}:{$tmpPath}"; } elseif (filled($this->customLocation)) { + // Validate the custom location to prevent command injection + if (! $this->validateServerPath($this->customLocation)) { + $this->dispatch('error', 'Invalid file path. Path must be absolute and contain only safe characters.'); + + return; + } $tmpPath = '/tmp/restore_'.$this->resource->uuid; - $this->importCommands[] = "docker cp {$this->customLocation} {$this->container}:{$tmpPath}"; + $escapedCustomLocation = escapeshellarg($this->customLocation); + $this->importCommands[] = "docker cp {$escapedCustomLocation} {$this->container}:{$tmpPath}"; } else { $this->dispatch('error', 'The file does not exist or has been deleted.'); @@ -208,38 +309,7 @@ public function runImport() // Copy the restore command to a script file $scriptPath = "/tmp/restore_{$this->resource->uuid}.sh"; - switch ($this->resource->getMorphClass()) { - case \App\Models\StandaloneMariadb::class: - $restoreCommand = $this->mariadbRestoreCommand; - if ($this->dumpAll) { - $restoreCommand .= " && (gunzip -cf {$tmpPath} 2>/dev/null || cat {$tmpPath}) | mariadb -u root -p\$MARIADB_ROOT_PASSWORD"; - } else { - $restoreCommand .= " < {$tmpPath}"; - } - break; - case \App\Models\StandaloneMysql::class: - $restoreCommand = $this->mysqlRestoreCommand; - if ($this->dumpAll) { - $restoreCommand .= " && (gunzip -cf {$tmpPath} 2>/dev/null || cat {$tmpPath}) | mysql -u root -p\$MYSQL_ROOT_PASSWORD"; - } else { - $restoreCommand .= " < {$tmpPath}"; - } - break; - case \App\Models\StandalonePostgresql::class: - $restoreCommand = $this->postgresqlRestoreCommand; - if ($this->dumpAll) { - $restoreCommand .= " && (gunzip -cf {$tmpPath} 2>/dev/null || cat {$tmpPath}) | psql -U \$POSTGRES_USER postgres"; - } else { - $restoreCommand .= " {$tmpPath}"; - } - break; - case \App\Models\StandaloneMongodb::class: - $restoreCommand = $this->mongodbRestoreCommand; - if ($this->dumpAll === false) { - $restoreCommand .= "{$tmpPath}"; - } - break; - } + $restoreCommand = $this->buildRestoreCommand($tmpPath); $restoreCommandBase64 = base64_encode($restoreCommand); $this->importCommands[] = "echo \"{$restoreCommandBase64}\" | base64 -d > {$scriptPath}"; @@ -311,9 +381,26 @@ public function checkS3File() return; } + // Clean the path (remove leading slash if present) + $cleanPath = ltrim($this->s3Path, '/'); + + // Validate the S3 path early to prevent command injection in subsequent operations + if (! $this->validateS3Path($cleanPath)) { + $this->dispatch('error', 'Invalid S3 path. Path must contain only safe characters (alphanumerics, dots, dashes, underscores, slashes).'); + + return; + } + try { $s3Storage = S3Storage::ownedByCurrentTeam()->findOrFail($this->s3StorageId); + // Validate bucket name early + if (! $this->validateBucketName($s3Storage->bucket)) { + $this->dispatch('error', 'Invalid S3 bucket name. Bucket name must contain only alphanumerics, dots, dashes, and underscores.'); + + return; + } + // Test connection $s3Storage->testConnection(); @@ -328,9 +415,6 @@ public function checkS3File() 'use_path_style_endpoint' => true, ]); - // Clean the path (remove leading slash if present) - $cleanPath = ltrim($this->s3Path, '/'); - // Check if file exists if (! $disk->exists($cleanPath)) { $this->dispatch('error', 'File not found in S3. Please check the path.'); @@ -375,9 +459,23 @@ public function restoreFromS3() $bucket = $s3Storage->bucket; $endpoint = $s3Storage->endpoint; + // Validate bucket name to prevent command injection + if (! $this->validateBucketName($bucket)) { + $this->dispatch('error', 'Invalid S3 bucket name. Bucket name must contain only alphanumerics, dots, dashes, and underscores.'); + + return; + } + // Clean the S3 path $cleanPath = ltrim($this->s3Path, '/'); + // Validate the S3 path to prevent command injection + if (! $this->validateS3Path($cleanPath)) { + $this->dispatch('error', 'Invalid S3 path. Path must contain only safe characters (alphanumerics, dots, dashes, underscores, slashes).'); + + return; + } + // Get helper image $helperImage = config('constants.coolify.helper_image'); $latestVersion = getHelperVersion(); @@ -410,11 +508,15 @@ public function restoreFromS3() $escapedSecret = escapeshellarg($secret); $commands[] = "docker exec {$containerName} mc alias set s3temp {$escapedEndpoint} {$escapedKey} {$escapedSecret}"; - // 4. Check file exists in S3 - $commands[] = "docker exec {$containerName} mc stat s3temp/{$bucket}/{$cleanPath}"; + // 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}"; // 5. Download from S3 to helper container (progress shown by default) - $commands[] = "docker exec {$containerName} mc cp s3temp/{$bucket}/{$cleanPath} {$helperTmpPath}"; + $escapedHelperTmpPath = escapeshellarg($helperTmpPath); + $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}"; diff --git a/app/Livewire/Storage/Form.php b/app/Livewire/Storage/Form.php index 63d9ce3da..d101d7b58 100644 --- a/app/Livewire/Storage/Form.php +++ b/app/Livewire/Storage/Form.php @@ -129,7 +129,7 @@ public function testConnection() $this->storage->refresh(); $this->isUsable = $this->storage->is_usable; - $this->dispatch('error', 'Failed to create storage.', $e->getMessage()); + $this->dispatch('error', 'Failed to test connection.', $e->getMessage()); } } diff --git a/app/Policies/InstanceSettingsPolicy.php b/app/Policies/InstanceSettingsPolicy.php new file mode 100644 index 000000000..a04f07a28 --- /dev/null +++ b/app/Policies/InstanceSettingsPolicy.php @@ -0,0 +1,25 @@ + \App\Policies\ApiTokenPolicy::class, + // Instance settings policy + \App\Models\InstanceSettings::class => \App\Policies\InstanceSettingsPolicy::class, + // Team policy \App\Models\Team::class => \App\Policies\TeamPolicy::class, diff --git a/resources/views/components/forms/env-var-input.blade.php b/resources/views/components/forms/env-var-input.blade.php index 0859db78d..833de7190 100644 --- a/resources/views/components/forms/env-var-input.blade.php +++ b/resources/views/components/forms/env-var-input.blade.php @@ -210,7 +210,7 @@ class="relative"> wire:loading.attr="disabled" type="{{ $type }}" @disabled($disabled) - @if ($htmlId !== 'null') id={{ $htmlId }} @endif + @if ($htmlId !== 'null') id="{{ $htmlId }}" @endif name="{{ $name }}" placeholder="{{ $attributes->get('placeholder') }}" @if ($autofocus) autofocus @endif> diff --git a/resources/views/livewire/project/database/import.blade.php b/resources/views/livewire/project/database/import.blade.php index b3c21e93e..17efa724f 100644 --- a/resources/views/livewire/project/database/import.blade.php +++ b/resources/views/livewire/project/database/import.blade.php @@ -148,7 +148,7 @@ class="flex-1 p-6 border-2 rounded-sm cursor-pointer transition-all"

File Information

-
Location: /
+
Location:
diff --git a/resources/views/livewire/settings/index.blade.php b/resources/views/livewire/settings/index.blade.php index 85c151399..deba90291 100644 --- a/resources/views/livewire/settings/index.blade.php +++ b/resources/views/livewire/settings/index.blade.php @@ -9,7 +9,7 @@ class="flex flex-col h-full gap-8 sm:flex-row">

General

- + Save
@@ -18,10 +18,10 @@ class="flex flex-col h-full gap-8 sm:flex-row">
- -
- -
@@ -86,11 +86,9 @@ class="px-4 py-2 text-gray-800 cursor-pointer hover:bg-gray-100 dark:hover:bg-co
@endif @if(isDev()) - - -
+ @endif
diff --git a/tests/Unit/Project/Database/ImportCheckFileButtonTest.php b/tests/Unit/Project/Database/ImportCheckFileButtonTest.php index 900cf02a4..a305160c0 100644 --- a/tests/Unit/Project/Database/ImportCheckFileButtonTest.php +++ b/tests/Unit/Project/Database/ImportCheckFileButtonTest.php @@ -37,3 +37,92 @@ expect($component->customLocation)->toBe(''); }); + +test('validateBucketName accepts valid bucket names', function () { + $component = new Import; + $method = new ReflectionMethod($component, 'validateBucketName'); + + // Valid bucket names + expect($method->invoke($component, 'my-bucket'))->toBeTrue(); + expect($method->invoke($component, 'my_bucket'))->toBeTrue(); + expect($method->invoke($component, 'mybucket123'))->toBeTrue(); + expect($method->invoke($component, 'my.bucket.name'))->toBeTrue(); + expect($method->invoke($component, 'Bucket-Name_123'))->toBeTrue(); +}); + +test('validateBucketName rejects invalid bucket names', function () { + $component = new Import; + $method = new ReflectionMethod($component, 'validateBucketName'); + + // Invalid bucket names (command injection attempts) + expect($method->invoke($component, 'bucket;rm -rf /'))->toBeFalse(); + expect($method->invoke($component, 'bucket$(whoami)'))->toBeFalse(); + expect($method->invoke($component, 'bucket`id`'))->toBeFalse(); + expect($method->invoke($component, 'bucket|cat /etc/passwd'))->toBeFalse(); + expect($method->invoke($component, 'bucket&ls'))->toBeFalse(); + expect($method->invoke($component, "bucket\nid"))->toBeFalse(); + expect($method->invoke($component, 'bucket name'))->toBeFalse(); // Space not allowed in bucket +}); + +test('validateS3Path accepts valid S3 paths', function () { + $component = new Import; + $method = new ReflectionMethod($component, 'validateS3Path'); + + // Valid S3 paths + expect($method->invoke($component, 'backup.sql'))->toBeTrue(); + expect($method->invoke($component, 'folder/backup.sql'))->toBeTrue(); + expect($method->invoke($component, 'my-folder/my_backup.sql.gz'))->toBeTrue(); + expect($method->invoke($component, 'path/to/deep/file.tar.gz'))->toBeTrue(); + expect($method->invoke($component, 'folder with space/file.sql'))->toBeTrue(); +}); + +test('validateS3Path rejects invalid S3 paths', function () { + $component = new Import; + $method = new ReflectionMethod($component, 'validateS3Path'); + + // Invalid S3 paths (command injection attempts) + expect($method->invoke($component, ''))->toBeFalse(); // Empty + expect($method->invoke($component, '../etc/passwd'))->toBeFalse(); // Directory traversal + expect($method->invoke($component, 'path;rm -rf /'))->toBeFalse(); + expect($method->invoke($component, 'path$(whoami)'))->toBeFalse(); + expect($method->invoke($component, 'path`id`'))->toBeFalse(); + expect($method->invoke($component, 'path|cat /etc/passwd'))->toBeFalse(); + expect($method->invoke($component, 'path&ls'))->toBeFalse(); + expect($method->invoke($component, "path\nid"))->toBeFalse(); + expect($method->invoke($component, "path\r\nid"))->toBeFalse(); + expect($method->invoke($component, "path\0id"))->toBeFalse(); // Null byte + expect($method->invoke($component, "path'injection"))->toBeFalse(); + expect($method->invoke($component, 'path"injection'))->toBeFalse(); + expect($method->invoke($component, 'path\\injection'))->toBeFalse(); +}); + +test('validateServerPath accepts valid server paths', function () { + $component = new Import; + $method = new ReflectionMethod($component, 'validateServerPath'); + + // Valid server paths (must be absolute) + expect($method->invoke($component, '/tmp/backup.sql'))->toBeTrue(); + expect($method->invoke($component, '/var/backups/my-backup.sql'))->toBeTrue(); + expect($method->invoke($component, '/home/user/data_backup.sql.gz'))->toBeTrue(); + expect($method->invoke($component, '/path/to/deep/nested/file.tar.gz'))->toBeTrue(); +}); + +test('validateServerPath rejects invalid server paths', function () { + $component = new Import; + $method = new ReflectionMethod($component, 'validateServerPath'); + + // Invalid server paths + expect($method->invoke($component, 'relative/path.sql'))->toBeFalse(); // Not absolute + expect($method->invoke($component, '/path/../etc/passwd'))->toBeFalse(); // Directory traversal + expect($method->invoke($component, '/path;rm -rf /'))->toBeFalse(); + expect($method->invoke($component, '/path$(whoami)'))->toBeFalse(); + expect($method->invoke($component, '/path`id`'))->toBeFalse(); + expect($method->invoke($component, '/path|cat /etc/passwd'))->toBeFalse(); + expect($method->invoke($component, '/path&ls'))->toBeFalse(); + expect($method->invoke($component, "/path\nid"))->toBeFalse(); + expect($method->invoke($component, "/path\r\nid"))->toBeFalse(); + expect($method->invoke($component, "/path\0id"))->toBeFalse(); // Null byte + expect($method->invoke($component, "/path'injection"))->toBeFalse(); + expect($method->invoke($component, '/path"injection'))->toBeFalse(); + expect($method->invoke($component, '/path\\injection'))->toBeFalse(); +});