diff --git a/app/Models/S3Storage.php b/app/Models/S3Storage.php index 47652eb35..3aae55966 100644 --- a/app/Models/S3Storage.php +++ b/app/Models/S3Storage.php @@ -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 @@ protected function path(): Attribute ); } + /** + * 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 { diff --git a/database/migrations/2025_12_15_143052_trim_s3_storage_credentials.php b/database/migrations/2025_12_15_143052_trim_s3_storage_credentials.php new file mode 100644 index 000000000..bb59d7358 --- /dev/null +++ b/database/migrations/2025_12_15_143052_trim_s3_storage_credentials.php @@ -0,0 +1,112 @@ +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 + } +};