This commit is contained in:
Karl Bauer 2026-03-10 10:17:52 -06:00 committed by GitHub
commit 74ff84c7fe
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 139 additions and 2 deletions

View file

@ -666,7 +666,21 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
$escapedSecret = escapeshellarg($secret);
$commands[] = "docker exec backup-of-{$this->backup_log_uuid} mc alias set temporary {$escapedEndpoint} {$escapedKey} {$escapedSecret}";
$commands[] = "docker exec backup-of-{$this->backup_log_uuid} mc cp $this->backup_location temporary/$bucket{$this->backup_dir}/";
// Build S3 path with optional prefix
$s3Path = $bucket;
if (filled($this->s3->path)) {
// Strip leading/trailing slashes to avoid double slashes in path
$pathPrefix = trim($this->s3->path, '/');
$s3Path .= '/'.$pathPrefix;
}
$s3Path .= $this->backup_dir.'/';
// Escape the paths to prevent command injection
$escapedBackupLocation = escapeshellarg($this->backup_location);
$escapedS3Path = escapeshellarg("temporary/{$s3Path}");
$commands[] = "docker exec backup-of-{$this->backup_log_uuid} mc cp {$escapedBackupLocation} {$escapedS3Path}";
instant_remote_process($commands, $this->server, true, false, null, disableMultiplexing: true);
$this->s3_uploaded = true;

View file

@ -29,6 +29,8 @@ class Form extends Component
public string $secret;
public ?string $path = null;
public ?bool $isUsable = null;
protected function rules(): array
@ -42,6 +44,7 @@ class Form extends Component
'secret' => 'required|max:255',
'bucket' => 'required|max:255',
'endpoint' => 'required|url|max:255',
'path' => ['nullable', 'max:255', 'regex:/^[a-zA-Z0-9\/\-_\.]*$/', 'not_regex:/\.\./'],
];
}
@ -61,6 +64,9 @@ class Form extends Component
'endpoint.required' => 'The Endpoint field is required.',
'endpoint.url' => 'The Endpoint must be a valid URL.',
'endpoint.max' => 'The Endpoint may not be greater than 255 characters.',
'path.max' => 'The Path Prefix may not be greater than 255 characters.',
'path.regex' => 'The Path Prefix may only contain letters, numbers, slashes (/), dashes (-), underscores (_), and dots (.).',
'path.not_regex' => 'The Path Prefix may not contain directory traversal sequences (..).',
]
);
}
@ -74,6 +80,7 @@ class Form extends Component
'secret' => 'Secret',
'bucket' => 'Bucket',
'endpoint' => 'Endpoint',
'path' => 'Path Prefix',
];
/**
@ -92,6 +99,7 @@ class Form extends Component
$this->storage->region = $this->region;
$this->storage->key = $this->key;
$this->storage->secret = $this->secret;
$this->storage->path = $this->path;
$this->storage->is_usable = $this->isUsable;
} else {
// Sync FROM model (on load/refresh)
@ -102,6 +110,7 @@ class Form extends Component
$this->region = $this->storage->region;
$this->key = $this->storage->key;
$this->secret = $this->storage->secret;
$this->path = $this->storage->path;
$this->isUsable = $this->storage->is_usable;
}
}

View file

@ -72,7 +72,11 @@ class S3Storage extends BaseModel
return null;
}
return str($value)->trim()->start('/')->value();
// Trim whitespace and normalize: add leading slash, remove trailing slash
$path = str($value)->trim()->start('/')->rtrim('/')->value();
// Return null if only slashes remain after trimming
return $path === '/' ? null : $path;
}
);
}

View file

@ -209,6 +209,19 @@ function deleteBackupsS3(string|array|null $filenames, S3Storage $s3): void
$filenames = [$filenames];
}
// Apply path prefix if configured
if (filled($s3->path)) {
// Strip leading/trailing slashes to avoid double slashes in path
$pathPrefix = trim($s3->path, '/');
$filenames = array_map(function ($filename) use ($pathPrefix) {
// The filename is the backup path (e.g., /data/coolify/backups/databases/...)
// We need to prepend the path prefix to match the S3 key structure
$cleanFilename = ltrim($filename, '/');
return $pathPrefix.'/'.$cleanFilename;
}, $filenames);
}
$disk = Storage::build([
'driver' => 's3',
'key' => $s3->key,

View file

@ -0,0 +1,28 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
return new class extends Migration
{
/**
* Run the migrations.
*/
public function up(): void
{
Schema::table('s3_storages', function (Blueprint $table) {
$table->string('path')->nullable()->after('endpoint');
});
}
/**
* Reverse the migrations.
*/
public function down(): void
{
Schema::table('s3_storages', function (Blueprint $table) {
$table->dropColumn('path');
});
}
};

View file

@ -40,6 +40,11 @@
<x-forms.input canGate="update" :canResource="$storage" required label="Bucket" id="bucket" />
<x-forms.input canGate="update" :canResource="$storage" required label="Region" id="region" />
</div>
<div class="flex gap-2">
<x-forms.input canGate="update" :canResource="$storage" label="Path Prefix" id="path"
placeholder="e.g., production or instance-1"
helper="Optional prefix added before the standard backup path (/data/coolify/backups/...). Useful for separating multiple Coolify instances in a single bucket." />
</div>
<div class="flex gap-2">
<x-forms.input canGate="update" :canResource="$storage" required type="password" label="Access Key"
id="key" />

View file

@ -51,3 +51,67 @@ test('S3Storage model is guarded correctly', function () {
// The model should have $guarded = [] which means everything is fillable
expect($s3Storage->getGuarded())->toBe([]);
});
test('S3Storage path attribute normalizes path correctly', function () {
$s3Storage = new S3Storage;
// Path should be normalized to start with /
$s3Storage->path = 'backups/coolify';
expect($s3Storage->path)->toBe('/backups/coolify');
// Path with leading slash should remain unchanged
$s3Storage->path = '/backups/coolify';
expect($s3Storage->path)->toBe('/backups/coolify');
// Empty path should return null
$s3Storage->path = '';
expect($s3Storage->path)->toBeNull();
// Null path should return null
$s3Storage->path = null;
expect($s3Storage->path)->toBeNull();
// Path with whitespace should be trimmed
$s3Storage->path = ' backups/coolify ';
expect($s3Storage->path)->toBe('/backups/coolify');
});
test('S3Storage path attribute handles various path formats', function () {
$s3Storage = new S3Storage;
// Simple path
$s3Storage->path = 'instance-1';
expect($s3Storage->path)->toBe('/instance-1');
// Nested path
$s3Storage->path = 'production/backups/db';
expect($s3Storage->path)->toBe('/production/backups/db');
// Path with special characters
$s3Storage->path = 'my-instance_2024.backups';
expect($s3Storage->path)->toBe('/my-instance_2024.backups');
});
test('S3Storage path attribute handles edge cases', function () {
$s3Storage = new S3Storage;
// Multiple consecutive slashes are preserved (validation should catch this)
$s3Storage->path = 'path//to///backup';
expect($s3Storage->path)->toBe('/path//to///backup');
// Path ending with slash - trailing slashes are now stripped
$s3Storage->path = 'backups/coolify/';
expect($s3Storage->path)->toBe('/backups/coolify');
// Only whitespace should return null
$s3Storage->path = ' ';
expect($s3Storage->path)->toBeNull();
// Path with dots (valid single dots)
$s3Storage->path = 'path.with.dots';
expect($s3Storage->path)->toBe('/path.with.dots');
// Only slashes should return null
$s3Storage->path = '///';
expect($s3Storage->path)->toBeNull();
});