Skip to content

Commit c8e2cf4

Browse files
committed
fix: add realpath error handling, temp cleanup on fatal errors, credential sanitization, and
robust port validation
1 parent 1ea0079 commit c8e2cf4

File tree

4 files changed

+88
-11
lines changed

4 files changed

+88
-11
lines changed
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
namespace Spatie\Backup\Helpers;
4+
5+
class CredentialSanitizer
6+
{
7+
/**
8+
* Sanitize exception messages to remove sensitive credentials.
9+
*/
10+
public static function sanitizeMessage(string $message): string
11+
{
12+
// Pattern to match common credential formats in connection strings and URLs
13+
$patterns = [
14+
// MySQL/PostgreSQL connection strings: password=secret or password='secret' or password="secret"
15+
'/password\s*=\s*["\']?[^"\'\s;]+["\']?/i' => 'password=***',
16+
// URLs with credentials: user:password@host
17+
'/:([^:@\s]+)@/i' => ':***@',
18+
// Environment variable patterns
19+
'/DB_PASSWORD\s*=\s*.+/i' => 'DB_PASSWORD=***',
20+
];
21+
22+
foreach ($patterns as $pattern => $replacement) {
23+
$message = preg_replace($pattern, $replacement, $message);
24+
}
25+
26+
return $message;
27+
}
28+
29+
/**
30+
* Sanitize exception object by replacing its message with a sanitized version.
31+
*/
32+
public static function sanitizeException(\Throwable $exception): string
33+
{
34+
$sanitizedMessage = self::sanitizeMessage($exception->getMessage());
35+
$trace = self::sanitizeStackTrace($exception->getTraceAsString());
36+
37+
return $sanitizedMessage . PHP_EOL . $trace;
38+
}
39+
40+
/**
41+
* Sanitize stack trace to remove credentials.
42+
*/
43+
protected static function sanitizeStackTrace(string $trace): string
44+
{
45+
return self::sanitizeMessage($trace);
46+
}
47+
}

src/Tasks/Backup/BackupJob.php

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
use Spatie\Backup\Events\DumpingDatabase;
1717
use Spatie\Backup\Exceptions\BackupFailed;
1818
use Spatie\Backup\Exceptions\InvalidBackupJob;
19+
use Spatie\Backup\Helpers\CredentialSanitizer;
20+
use Spatie\DbDumper\Compressors\GzipCompressor;
1921
use Spatie\DbDumper\Databases\MongoDb;
2022
use Spatie\DbDumper\Databases\Sqlite;
2123
use Spatie\DbDumper\DbDumper;
@@ -155,6 +157,14 @@ public function run(): void
155157
->force()
156158
->create()
157159
->empty();
160+
$cleanupRegistered = false;
161+
$shutdownHandler = function () use (&$cleanupRegistered) {
162+
if ($cleanupRegistered && $this->temporaryDirectory->exists()) {
163+
$this->temporaryDirectory->delete();
164+
}
165+
};
166+
register_shutdown_function($shutdownHandler);
167+
$cleanupRegistered = true;
158168

159169
if ($this->signals) {
160170
Signal::handle(SIGINT, function (Command $command) {
@@ -179,14 +189,16 @@ public function run(): void
179189

180190
$this->copyToBackupDestinations($zipFile);
181191
} catch (Exception $exception) {
182-
consoleOutput()->error("Backup failed because: {$exception->getMessage()}.".PHP_EOL.$exception->getTraceAsString());
192+
$sanitizedError = CredentialSanitizer::sanitizeException($exception);
193+
consoleOutput()->error("Backup failed because: {$sanitizedError}");
183194

184195
$this->temporaryDirectory->delete();
185196

186197
throw BackupFailed::from($exception);
187198
}
188199

189200
$this->temporaryDirectory->delete();
201+
$cleanupRegistered = false; // Prevent double cleanup
190202

191203
if ($this->signals) {
192204
Signal::clearHandlers(SIGINT);
@@ -302,7 +314,8 @@ protected function copyToBackupDestinations(string $path): void
302314
->each(function (BackupDestination $backupDestination) use ($path) {
303315
try {
304316
if (! $backupDestination->isReachable()) {
305-
throw new Exception("Could not connect to disk {$backupDestination->diskName()} because: {$backupDestination->connectionError()}");
317+
$sanitizedError = CredentialSanitizer::sanitizeMessage($backupDestination->connectionError());
318+
throw new Exception("Could not connect to disk {$backupDestination->diskName()} because: {$sanitizedError}");
306319
}
307320

308321
consoleOutput()->info("Copying zip to disk named {$backupDestination->diskName()}...");
@@ -313,7 +326,8 @@ protected function copyToBackupDestinations(string $path): void
313326

314327
$this->sendNotification(new BackupWasSuccessful($backupDestination));
315328
} catch (Exception $exception) {
316-
consoleOutput()->error("Copying zip failed because: {$exception->getMessage()}.");
329+
$sanitizedError = CredentialSanitizer::sanitizeMessage($exception->getMessage());
330+
consoleOutput()->error("Copying zip failed because: {$sanitizedError}");
317331

318332
throw BackupFailed::from($exception)->destination($backupDestination);
319333
}

src/Tasks/Backup/DbDumperFactory.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,17 @@ public static function createFromConnection(string $dbConnectionName): DbDumper
5858
}
5959

6060
if (isset($dbConfig['port'])) {
61-
if (filter_var($dbConfig['port'], FILTER_VALIDATE_INT, [
61+
$port = $dbConfig['port'];
62+
if ($port === '' || $port === null) {
63+
} elseif (filter_var($port, FILTER_VALIDATE_INT, [
6264
'options' => [
6365
'min_range' => 1,
6466
'max_range' => 65535,
6567
],
6668
]) !== false) {
67-
$dbDumper = $dbDumper->setPort((int) $dbConfig['port']);
69+
$dbDumper = $dbDumper->setPort((int) $port);
70+
} else {
71+
consoleOutput()->warn("Invalid port value '{$port}' for database connection '{$dbConnectionName}'. Using default port.");
6872
}
6973
}
7074

src/Tasks/Backup/FileSelection.php

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,14 +111,20 @@ protected function includedDirectories(): array
111111

112112
protected function shouldExclude(string $path): bool
113113
{
114-
$path = realpath($path);
115-
if (is_dir($path)) {
116-
$path .= DIRECTORY_SEPARATOR;
114+
$realPath = realpath($path);
115+
116+
if ($realPath === false) {
117+
consoleOutput()->warn("Cannot resolve path: {$path}. Skipping...");
118+
return false;
119+
}
120+
121+
if (is_dir($realPath)) {
122+
$realPath .= DIRECTORY_SEPARATOR;
117123
}
118124

119125
foreach ($this->excludeFilesAndDirectories as $excludedPath) {
120-
if (Str::startsWith($path, $excludedPath.(is_dir($excludedPath) ? DIRECTORY_SEPARATOR : ''))) {
121-
if ($path != $excludedPath && is_file($excludedPath)) {
126+
if (Str::startsWith($realPath, $excludedPath.(is_dir($excludedPath) ? DIRECTORY_SEPARATOR : ''))) {
127+
if ($realPath != $excludedPath && is_file($excludedPath)) {
122128
continue;
123129
}
124130

@@ -137,7 +143,13 @@ protected function sanitize(string|array $paths): Collection
137143
return collect($paths)
138144
->reject(fn (string $path) => $path === '')
139145
->flatMap(fn (string $path) => $this->getMatchingPaths($path))
140-
->map(fn (string $path) => realpath($path))
146+
->map(function (string $path) {
147+
$realPath = realpath($path);
148+
if ($realPath === false) {
149+
consoleOutput()->warn("Cannot resolve path: {$path}. This path will be excluded from backup.");
150+
}
151+
return $realPath;
152+
})
141153
->reject(fn ($path) => $path === false);
142154
}
143155

0 commit comments

Comments
 (0)