Fix S3 credential whitespace issue with proper trimming (#7638)

This commit is contained in:
Andras Bacsai 2025-12-15 12:23:20 +01:00 committed by GitHub
commit 383572edac
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 164 additions and 0 deletions

View File

@ -20,6 +20,28 @@ class S3Storage extends BaseModel
'secret' => 'encrypted',
];
/**
* Boot the model and register event listeners.
*/
protected static function boot(): void
{
parent::boot();
// Trim whitespace from credentials before saving to prevent
// "Malformed Access Key Id" errors from accidental whitespace in pasted values.
// Note: We use the saving event instead of Attribute mutators because key/secret
// use Laravel's 'encrypted' cast. Attribute mutators fire before casts, which
// would cause issues with the encryption/decryption cycle.
static::saving(function (S3Storage $storage) {
if ($storage->key !== null) {
$storage->key = trim($storage->key);
}
if ($storage->secret !== null) {
$storage->secret = trim($storage->secret);
}
});
}
public static function ownedByCurrentTeam(array $select = ['*'])
{
$selectArray = collect($select)->concat(['id']);
@ -55,6 +77,36 @@ class S3Storage extends BaseModel
);
}
/**
* Trim whitespace from endpoint to prevent malformed URLs.
*/
protected function endpoint(): Attribute
{
return Attribute::make(
set: fn (?string $value) => $value ? trim($value) : null,
);
}
/**
* Trim whitespace from bucket name to prevent connection errors.
*/
protected function bucket(): Attribute
{
return Attribute::make(
set: fn (?string $value) => $value ? trim($value) : null,
);
}
/**
* Trim whitespace from region to prevent connection errors.
*/
protected function region(): Attribute
{
return Attribute::make(
set: fn (?string $value) => $value ? trim($value) : null,
);
}
public function testConnection(bool $shouldSave = false)
{
try {

View File

@ -0,0 +1,112 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Support\Facades\Crypt;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Log;
return new class extends Migration
{
/**
* Run the migrations.
*
* Trims whitespace from S3 storage fields (key, secret, endpoint, bucket, region)
* to fix "Malformed Access Key Id" errors that can occur when users
* accidentally paste values with leading/trailing whitespace.
*/
public function up(): void
{
DB::table('s3_storages')
->select(['id', 'key', 'secret', 'endpoint', 'bucket', 'region'])
->orderBy('id')
->chunk(100, function ($storages) {
foreach ($storages as $storage) {
try {
DB::transaction(function () use ($storage) {
$updates = [];
// Trim endpoint (not encrypted)
if ($storage->endpoint !== null) {
$trimmedEndpoint = trim($storage->endpoint);
if ($trimmedEndpoint !== $storage->endpoint) {
$updates['endpoint'] = $trimmedEndpoint;
}
}
// Trim bucket (not encrypted)
if ($storage->bucket !== null) {
$trimmedBucket = trim($storage->bucket);
if ($trimmedBucket !== $storage->bucket) {
$updates['bucket'] = $trimmedBucket;
}
}
// Trim region (not encrypted)
if ($storage->region !== null) {
$trimmedRegion = trim($storage->region);
if ($trimmedRegion !== $storage->region) {
$updates['region'] = $trimmedRegion;
}
}
// Trim key (encrypted) - verify re-encryption works before saving
if ($storage->key !== null) {
try {
$decryptedKey = Crypt::decryptString($storage->key);
$trimmedKey = trim($decryptedKey);
if ($trimmedKey !== $decryptedKey) {
$encryptedKey = Crypt::encryptString($trimmedKey);
// Verify the new encryption is valid
if (Crypt::decryptString($encryptedKey) === $trimmedKey) {
$updates['key'] = $encryptedKey;
} else {
Log::warning("S3 storage ID {$storage->id}: Re-encryption verification failed for key, skipping");
}
}
} catch (\Throwable $e) {
Log::warning("Could not decrypt S3 storage key for ID {$storage->id}: ".$e->getMessage());
}
}
// Trim secret (encrypted) - verify re-encryption works before saving
if ($storage->secret !== null) {
try {
$decryptedSecret = Crypt::decryptString($storage->secret);
$trimmedSecret = trim($decryptedSecret);
if ($trimmedSecret !== $decryptedSecret) {
$encryptedSecret = Crypt::encryptString($trimmedSecret);
// Verify the new encryption is valid
if (Crypt::decryptString($encryptedSecret) === $trimmedSecret) {
$updates['secret'] = $encryptedSecret;
} else {
Log::warning("S3 storage ID {$storage->id}: Re-encryption verification failed for secret, skipping");
}
}
} catch (\Throwable $e) {
Log::warning("Could not decrypt S3 storage secret for ID {$storage->id}: ".$e->getMessage());
}
}
if (! empty($updates)) {
DB::table('s3_storages')->where('id', $storage->id)->update($updates);
Log::info("Trimmed whitespace from S3 storage credentials for ID {$storage->id}", [
'fields_updated' => array_keys($updates),
]);
}
});
} catch (\Throwable $e) {
Log::error("Failed to process S3 storage ID {$storage->id}: ".$e->getMessage());
// Continue with next record instead of failing entire migration
}
}
});
}
/**
* Reverse the migrations.
*/
public function down(): void
{
// Cannot reverse trimming operation
}
};