From c8e2cf43d617a2516af75dedbfd4c7ec724dd3b4 Mon Sep 17 00:00:00 2001 From: Hayatunnabi Nabil Date: Wed, 15 Oct 2025 10:17:21 +0600 Subject: [PATCH 1/2] fix: add realpath error handling, temp cleanup on fatal errors, credential sanitization, and robust port validation --- src/Helpers/CredentialSanitizer.php | 47 ++++++++++++++++++++++++++++ src/Tasks/Backup/BackupJob.php | 20 ++++++++++-- src/Tasks/Backup/DbDumperFactory.php | 8 +++-- src/Tasks/Backup/FileSelection.php | 24 ++++++++++---- 4 files changed, 88 insertions(+), 11 deletions(-) create mode 100644 src/Helpers/CredentialSanitizer.php diff --git a/src/Helpers/CredentialSanitizer.php b/src/Helpers/CredentialSanitizer.php new file mode 100644 index 00000000..abc9a49c --- /dev/null +++ b/src/Helpers/CredentialSanitizer.php @@ -0,0 +1,47 @@ + 'password=***', + // URLs with credentials: user:password@host + '/:([^:@\s]+)@/i' => ':***@', + // Environment variable patterns + '/DB_PASSWORD\s*=\s*.+/i' => 'DB_PASSWORD=***', + ]; + + foreach ($patterns as $pattern => $replacement) { + $message = preg_replace($pattern, $replacement, $message); + } + + return $message; + } + + /** + * Sanitize exception object by replacing its message with a sanitized version. + */ + public static function sanitizeException(\Throwable $exception): string + { + $sanitizedMessage = self::sanitizeMessage($exception->getMessage()); + $trace = self::sanitizeStackTrace($exception->getTraceAsString()); + + return $sanitizedMessage . PHP_EOL . $trace; + } + + /** + * Sanitize stack trace to remove credentials. + */ + protected static function sanitizeStackTrace(string $trace): string + { + return self::sanitizeMessage($trace); + } +} diff --git a/src/Tasks/Backup/BackupJob.php b/src/Tasks/Backup/BackupJob.php index a556414b..59da6a88 100644 --- a/src/Tasks/Backup/BackupJob.php +++ b/src/Tasks/Backup/BackupJob.php @@ -16,6 +16,8 @@ use Spatie\Backup\Events\DumpingDatabase; use Spatie\Backup\Exceptions\BackupFailed; use Spatie\Backup\Exceptions\InvalidBackupJob; +use Spatie\Backup\Helpers\CredentialSanitizer; +use Spatie\DbDumper\Compressors\GzipCompressor; use Spatie\DbDumper\Databases\MongoDb; use Spatie\DbDumper\Databases\Sqlite; use Spatie\DbDumper\DbDumper; @@ -155,6 +157,14 @@ public function run(): void ->force() ->create() ->empty(); + $cleanupRegistered = false; + $shutdownHandler = function () use (&$cleanupRegistered) { + if ($cleanupRegistered && $this->temporaryDirectory->exists()) { + $this->temporaryDirectory->delete(); + } + }; + register_shutdown_function($shutdownHandler); + $cleanupRegistered = true; if ($this->signals) { Signal::handle(SIGINT, function (Command $command) { @@ -179,7 +189,8 @@ public function run(): void $this->copyToBackupDestinations($zipFile); } catch (Exception $exception) { - consoleOutput()->error("Backup failed because: {$exception->getMessage()}.".PHP_EOL.$exception->getTraceAsString()); + $sanitizedError = CredentialSanitizer::sanitizeException($exception); + consoleOutput()->error("Backup failed because: {$sanitizedError}"); $this->temporaryDirectory->delete(); @@ -187,6 +198,7 @@ public function run(): void } $this->temporaryDirectory->delete(); + $cleanupRegistered = false; // Prevent double cleanup if ($this->signals) { Signal::clearHandlers(SIGINT); @@ -302,7 +314,8 @@ protected function copyToBackupDestinations(string $path): void ->each(function (BackupDestination $backupDestination) use ($path) { try { if (! $backupDestination->isReachable()) { - throw new Exception("Could not connect to disk {$backupDestination->diskName()} because: {$backupDestination->connectionError()}"); + $sanitizedError = CredentialSanitizer::sanitizeMessage($backupDestination->connectionError()); + throw new Exception("Could not connect to disk {$backupDestination->diskName()} because: {$sanitizedError}"); } consoleOutput()->info("Copying zip to disk named {$backupDestination->diskName()}..."); @@ -313,7 +326,8 @@ protected function copyToBackupDestinations(string $path): void $this->sendNotification(new BackupWasSuccessful($backupDestination)); } catch (Exception $exception) { - consoleOutput()->error("Copying zip failed because: {$exception->getMessage()}."); + $sanitizedError = CredentialSanitizer::sanitizeMessage($exception->getMessage()); + consoleOutput()->error("Copying zip failed because: {$sanitizedError}"); throw BackupFailed::from($exception)->destination($backupDestination); } diff --git a/src/Tasks/Backup/DbDumperFactory.php b/src/Tasks/Backup/DbDumperFactory.php index a7e0e080..40f2d617 100644 --- a/src/Tasks/Backup/DbDumperFactory.php +++ b/src/Tasks/Backup/DbDumperFactory.php @@ -58,13 +58,17 @@ public static function createFromConnection(string $dbConnectionName): DbDumper } if (isset($dbConfig['port'])) { - if (filter_var($dbConfig['port'], FILTER_VALIDATE_INT, [ + $port = $dbConfig['port']; + if ($port === '' || $port === null) { + } elseif (filter_var($port, FILTER_VALIDATE_INT, [ 'options' => [ 'min_range' => 1, 'max_range' => 65535, ], ]) !== false) { - $dbDumper = $dbDumper->setPort((int) $dbConfig['port']); + $dbDumper = $dbDumper->setPort((int) $port); + } else { + consoleOutput()->warn("Invalid port value '{$port}' for database connection '{$dbConnectionName}'. Using default port."); } } diff --git a/src/Tasks/Backup/FileSelection.php b/src/Tasks/Backup/FileSelection.php index e9325340..25563fe4 100644 --- a/src/Tasks/Backup/FileSelection.php +++ b/src/Tasks/Backup/FileSelection.php @@ -111,14 +111,20 @@ protected function includedDirectories(): array protected function shouldExclude(string $path): bool { - $path = realpath($path); - if (is_dir($path)) { - $path .= DIRECTORY_SEPARATOR; + $realPath = realpath($path); + + if ($realPath === false) { + consoleOutput()->warn("Cannot resolve path: {$path}. Skipping..."); + return false; + } + + if (is_dir($realPath)) { + $realPath .= DIRECTORY_SEPARATOR; } foreach ($this->excludeFilesAndDirectories as $excludedPath) { - if (Str::startsWith($path, $excludedPath.(is_dir($excludedPath) ? DIRECTORY_SEPARATOR : ''))) { - if ($path != $excludedPath && is_file($excludedPath)) { + if (Str::startsWith($realPath, $excludedPath.(is_dir($excludedPath) ? DIRECTORY_SEPARATOR : ''))) { + if ($realPath != $excludedPath && is_file($excludedPath)) { continue; } @@ -137,7 +143,13 @@ protected function sanitize(string|array $paths): Collection return collect($paths) ->reject(fn (string $path) => $path === '') ->flatMap(fn (string $path) => $this->getMatchingPaths($path)) - ->map(fn (string $path) => realpath($path)) + ->map(function (string $path) { + $realPath = realpath($path); + if ($realPath === false) { + consoleOutput()->warn("Cannot resolve path: {$path}. This path will be excluded from backup."); + } + return $realPath; + }) ->reject(fn ($path) => $path === false); } From e7f5adc5037f9db60e20e568066d754ca41c6b06 Mon Sep 17 00:00:00 2001 From: Hayatunnabi Nabil Date: Thu, 16 Oct 2025 17:31:43 +0600 Subject: [PATCH 2/2] feat: introduce CredentialExposure exception --- src/Exceptions/CredentialExposure.php | 13 ++++++++ src/Helpers/CredentialSanitizer.php | 47 --------------------------- src/Tasks/Backup/BackupJob.php | 10 ++---- 3 files changed, 16 insertions(+), 54 deletions(-) create mode 100644 src/Exceptions/CredentialExposure.php delete mode 100644 src/Helpers/CredentialSanitizer.php diff --git a/src/Exceptions/CredentialExposure.php b/src/Exceptions/CredentialExposure.php new file mode 100644 index 00000000..6a6a49e4 --- /dev/null +++ b/src/Exceptions/CredentialExposure.php @@ -0,0 +1,13 @@ +getMessage(), $exception->getCode(), $exception); + } +} diff --git a/src/Helpers/CredentialSanitizer.php b/src/Helpers/CredentialSanitizer.php deleted file mode 100644 index abc9a49c..00000000 --- a/src/Helpers/CredentialSanitizer.php +++ /dev/null @@ -1,47 +0,0 @@ - 'password=***', - // URLs with credentials: user:password@host - '/:([^:@\s]+)@/i' => ':***@', - // Environment variable patterns - '/DB_PASSWORD\s*=\s*.+/i' => 'DB_PASSWORD=***', - ]; - - foreach ($patterns as $pattern => $replacement) { - $message = preg_replace($pattern, $replacement, $message); - } - - return $message; - } - - /** - * Sanitize exception object by replacing its message with a sanitized version. - */ - public static function sanitizeException(\Throwable $exception): string - { - $sanitizedMessage = self::sanitizeMessage($exception->getMessage()); - $trace = self::sanitizeStackTrace($exception->getTraceAsString()); - - return $sanitizedMessage . PHP_EOL . $trace; - } - - /** - * Sanitize stack trace to remove credentials. - */ - protected static function sanitizeStackTrace(string $trace): string - { - return self::sanitizeMessage($trace); - } -} diff --git a/src/Tasks/Backup/BackupJob.php b/src/Tasks/Backup/BackupJob.php index 59da6a88..39661594 100644 --- a/src/Tasks/Backup/BackupJob.php +++ b/src/Tasks/Backup/BackupJob.php @@ -16,7 +16,6 @@ use Spatie\Backup\Events\DumpingDatabase; use Spatie\Backup\Exceptions\BackupFailed; use Spatie\Backup\Exceptions\InvalidBackupJob; -use Spatie\Backup\Helpers\CredentialSanitizer; use Spatie\DbDumper\Compressors\GzipCompressor; use Spatie\DbDumper\Databases\MongoDb; use Spatie\DbDumper\Databases\Sqlite; @@ -189,8 +188,7 @@ public function run(): void $this->copyToBackupDestinations($zipFile); } catch (Exception $exception) { - $sanitizedError = CredentialSanitizer::sanitizeException($exception); - consoleOutput()->error("Backup failed because: {$sanitizedError}"); + consoleOutput()->error("Backup failed because: {$exception->getMessage()}.".PHP_EOL.$exception->getTraceAsString()); $this->temporaryDirectory->delete(); @@ -314,8 +312,7 @@ protected function copyToBackupDestinations(string $path): void ->each(function (BackupDestination $backupDestination) use ($path) { try { if (! $backupDestination->isReachable()) { - $sanitizedError = CredentialSanitizer::sanitizeMessage($backupDestination->connectionError()); - throw new Exception("Could not connect to disk {$backupDestination->diskName()} because: {$sanitizedError}"); + throw new Exception("Could not connect to disk {$backupDestination->diskName()} because: {$backupDestination->connectionError()}"); } consoleOutput()->info("Copying zip to disk named {$backupDestination->diskName()}..."); @@ -326,8 +323,7 @@ protected function copyToBackupDestinations(string $path): void $this->sendNotification(new BackupWasSuccessful($backupDestination)); } catch (Exception $exception) { - $sanitizedError = CredentialSanitizer::sanitizeMessage($exception->getMessage()); - consoleOutput()->error("Copying zip failed because: {$sanitizedError}"); + consoleOutput()->error("Copying zip failed because: {$exception->getMessage()}."); throw BackupFailed::from($exception)->destination($backupDestination); }