fix(s3): allow uppercase letters in S3 bucket names
S3-compatible and legacy buckets may include uppercase letters. Relax the bucket name pattern, validation messages, and tests so those names are accepted while still rejecting invalid characters and formats.
This commit is contained in:
parent
3f1235158d
commit
d24ee35824
9 changed files with 23 additions and 20 deletions
|
|
@ -48,3 +48,7 @@ ## Confirm whether old reports still apply before changing code
|
|||
## Compare routing identity, not complete domain URLs
|
||||
- Domain-conflict checks must treat `http://host` and `https://host` as the same routing identity.
|
||||
- Reproduce reports with the exact stored schemes before stating that duplicate detection works.
|
||||
|
||||
## Verify reported fixes against the running development app
|
||||
- When a user asks for before-and-after verification, test the unchanged and fixed production code against the same Jean Run environment.
|
||||
- Cover each requested interface, such as UI and API, and record the exact URL, response, persisted state, and relevant logs.
|
||||
|
|
|
|||
|
|
@ -582,7 +582,7 @@ public function checkS3File()
|
|||
|
||||
// Validate bucket name early
|
||||
if (! $this->validateBucketName($s3Storage->bucket)) {
|
||||
$this->dispatch('error', 'Invalid S3 bucket name. Bucket name must contain only lowercase letters, numbers, dots, and dashes, and must follow S3 bucket naming rules.');
|
||||
$this->dispatch('error', 'Invalid S3 bucket name. Bucket name must contain only letters, numbers, dots, and dashes, and must follow S3 bucket naming rules.');
|
||||
|
||||
return;
|
||||
}
|
||||
|
|
@ -664,7 +664,7 @@ public function restoreFromS3(string $password = ''): bool|string
|
|||
|
||||
// Validate bucket name to prevent command injection
|
||||
if (! $this->validateBucketName($bucket)) {
|
||||
$this->dispatch('error', 'Invalid S3 bucket name. Bucket name must contain only lowercase letters, numbers, dots, and dashes, and must follow S3 bucket naming rules.');
|
||||
$this->dispatch('error', 'Invalid S3 bucket name. Bucket name must contain only letters, numbers, dots, and dashes, and must follow S3 bucket naming rules.');
|
||||
|
||||
return true;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@ class ValidS3BucketName implements ValidationRule
|
|||
public function validate(string $attribute, mixed $value, Closure $fail): void
|
||||
{
|
||||
if (! is_string($value) || ! ValidationPatterns::isValidS3BucketName($value)) {
|
||||
$fail('The :attribute must be a valid S3 bucket name: 3-63 lowercase letters, numbers, dots, or hyphens; start and end with a letter or number; no consecutive dots, dot-hyphen pairs, or IP address format.');
|
||||
$fail('The :attribute must be a valid S3 bucket name: 3-63 letters, numbers, dots, or hyphens; start and end with a letter or number; no consecutive dots, dot-hyphen pairs, or IP address format.');
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -96,11 +96,11 @@ class ValidationPatterns
|
|||
/**
|
||||
* Pattern for S3 bucket names.
|
||||
*
|
||||
* Bucket names must be 3-63 lowercase characters, start and end with a
|
||||
* letter or digit, and contain only lowercase letters, digits, dots, and
|
||||
* hyphens. Additional semantic checks live in isValidS3BucketName().
|
||||
* Bucket names must be 3-63 characters, start and end with a letter or
|
||||
* digit, and contain only letters, digits, dots, and hyphens. Uppercase
|
||||
* letters remain supported for legacy and S3-compatible buckets.
|
||||
*/
|
||||
public const S3_BUCKET_NAME_PATTERN = '/\A(?=.{3,63}\z)[a-z0-9][a-z0-9.-]*[a-z0-9]\z/';
|
||||
public const S3_BUCKET_NAME_PATTERN = '/\A(?=.{3,63}\z)[A-Za-z0-9][A-Za-z0-9.-]*[A-Za-z0-9]\z/';
|
||||
|
||||
/**
|
||||
* Pattern for Docker-compatible environment variable keys.
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@
|
|||
use App\Models\S3Storage;
|
||||
use App\Models\Team;
|
||||
use App\Models\User;
|
||||
use Illuminate\Filesystem\FilesystemAdapter;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\Log;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
|
|
@ -463,7 +464,7 @@ function validS3StoragePayload(array $overrides = []): array
|
|||
test('validates a working s3 storage connection', function () {
|
||||
$storage = createS3StorageForTeam($this->team);
|
||||
|
||||
$disk = Mockery::mock();
|
||||
$disk = Mockery::mock(FilesystemAdapter::class);
|
||||
$disk->expects('files')->once()->andReturn([]);
|
||||
Storage::expects('build')->once()->andReturn($disk);
|
||||
|
||||
|
|
@ -484,7 +485,7 @@ function validS3StoragePayload(array $overrides = []): array
|
|||
test('detects an invalid s3 storage connection', function () {
|
||||
$storage = createS3StorageForTeam($this->team, ['is_usable' => true]);
|
||||
|
||||
$disk = Mockery::mock();
|
||||
$disk = Mockery::mock(FilesystemAdapter::class);
|
||||
$disk->expects('files')
|
||||
->once()
|
||||
->andThrow(new RuntimeException('Access Denied'));
|
||||
|
|
@ -519,7 +520,7 @@ function validS3StoragePayload(array $overrides = []): array
|
|||
test('writes an audit log entry when validating storage', function () {
|
||||
$storage = createS3StorageForTeam($this->team, ['name' => 'Audit Storage']);
|
||||
|
||||
$disk = Mockery::mock();
|
||||
$disk = Mockery::mock(FilesystemAdapter::class);
|
||||
$disk->expects('files')->once()->andReturn([]);
|
||||
Storage::expects('build')->once()->andReturn($disk);
|
||||
|
||||
|
|
|
|||
|
|
@ -2,17 +2,15 @@
|
|||
|
||||
use App\Livewire\Storage\Form;
|
||||
use App\Models\S3Storage;
|
||||
use Illuminate\Filesystem\FilesystemAdapter;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
use Tests\TestCase;
|
||||
|
||||
uses(TestCase::class);
|
||||
|
||||
it('tests the S3 connection with the values currently entered in the form', function () {
|
||||
if (! defined('CURLOPT_RESOLVE')) {
|
||||
define('CURLOPT_RESOLVE', 10203);
|
||||
}
|
||||
|
||||
$disk = Mockery::mock();
|
||||
$disk = Mockery::mock(FilesystemAdapter::class);
|
||||
$disk->expects('files')->once()->andReturn([]);
|
||||
$testedConfig = null;
|
||||
|
||||
|
|
|
|||
|
|
@ -39,6 +39,7 @@
|
|||
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, 'Legacy-Bucket'))->toBeTrue();
|
||||
});
|
||||
|
||||
test('validateBucketName rejects invalid bucket names', function () {
|
||||
|
|
@ -54,7 +55,6 @@
|
|||
expect($method->invoke($component, "bucket\nid"))->toBeFalse();
|
||||
expect($method->invoke($component, 'bucket name'))->toBeFalse(); // Space not allowed in bucket
|
||||
expect($method->invoke($component, 'my_bucket'))->toBeFalse();
|
||||
expect($method->invoke($component, 'Bucket-Name'))->toBeFalse();
|
||||
expect($method->invoke($component, '192.168.1.1'))->toBeFalse();
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
<?php
|
||||
|
||||
use App\Models\S3Storage;
|
||||
use Illuminate\Filesystem\FilesystemAdapter;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
use Tests\TestCase;
|
||||
|
||||
|
|
@ -67,7 +68,7 @@
|
|||
});
|
||||
|
||||
test('S3Storage connection validation uses short s3 client timeouts', function () {
|
||||
$disk = Mockery::mock();
|
||||
$disk = Mockery::mock(FilesystemAdapter::class);
|
||||
$disk->expects('files')->once()->andReturn([]);
|
||||
|
||||
Storage::expects('build')
|
||||
|
|
@ -87,7 +88,7 @@
|
|||
'region' => 'us-east-1',
|
||||
'key' => null,
|
||||
'secret' => null,
|
||||
'bucket' => 'test-bucket',
|
||||
'bucket' => 'Test-Bucket',
|
||||
'endpoint' => 'https://s3.amazonaws.com',
|
||||
]);
|
||||
|
||||
|
|
@ -97,7 +98,7 @@
|
|||
});
|
||||
|
||||
test('S3Storage connection validation returns friendly timeout error', function () {
|
||||
$disk = Mockery::mock();
|
||||
$disk = Mockery::mock(FilesystemAdapter::class);
|
||||
$disk->expects('files')
|
||||
->once()
|
||||
->andThrow(new RuntimeException('cURL error 28: Operation timed out after 15000 milliseconds'));
|
||||
|
|
@ -142,5 +143,4 @@
|
|||
'backticks' => ['lab`id`'],
|
||||
'newline' => ["lab\nid"],
|
||||
'underscore' => ['lab_bucket'],
|
||||
'uppercase' => ['LabBucket'],
|
||||
]);
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ function validS3BucketNameRulePasses(string $bucket): bool
|
|||
})->with([
|
||||
'short' => ['abc'],
|
||||
'simple' => ['coolify-backups'],
|
||||
'legacy uppercase' => ['CoolifyBackups'],
|
||||
'dots' => ['coolify.backups'],
|
||||
'digits' => ['backup-123'],
|
||||
'max length' => [str_repeat('a', 63)],
|
||||
|
|
@ -28,7 +29,6 @@ function validS3BucketNameRulePasses(string $bucket): bool
|
|||
})->with([
|
||||
'too short' => ['ab'],
|
||||
'too long' => [str_repeat('a', 64)],
|
||||
'uppercase' => ['CoolifyBackups'],
|
||||
'underscore' => ['coolify_backups'],
|
||||
'leading hyphen' => ['-coolify-backups'],
|
||||
'trailing hyphen' => ['coolify-backups-'],
|
||||
|
|
|
|||
Loading…
Reference in a new issue