Files
projectsend/tests/Feature/Platform/ExternalStorageSettingsTest.php
denkfabrik-li 9af0d643b1 Keep the mail and storage credentials out of the boot-config cache
MailConfigApplier and ExternalStorageConfigApplier read their settings
through the `encrypted` casts -- decrypted -- and wrote the result into
the cache store with rememberForever(). The SMTP password, the S3 secret
access key and the whole GCS service account key file, private key
included, went in as plain text under a key that never expires.

The cache store encrypts nothing. On the store INSTALL.md documents for a
manual install (CACHE_STORE=database) and config/cache.php defaults to,
that is the `cache` table of the same database whose dump the `encrypted`
cast exists to survive. On redis it is the redis dump.

The rule already exists, two files away. MailOAuthConnection states it:

  Transports read this row fresh at send time -- tokens must never travel
  through the boot-config cache (see MailConfigApplier, which caches only
  readiness and the account address).

MailConfigApplier's own cache-key comment says the same thing about the
same array: what is deliberately NOT in the cached shape is tokens,
because neither readiness nor an address is a credential. The SMTP
password was in it anyway. SocialSettings::available() names both classes
outright as making the mistake.

So the credentials are read the way the tokens already are: from the row,
at the point that uses them. The cached array keeps everything that is
not a credential, and each applier reads its secret inside the branch
that configures a transport -- an installation on OAuth, on cloud, or one
that has never opened the Email or Storage screen reads nothing extra.

BootSettingsCache grows a second entry point rather than the callers
restating its rule. The cached read already survives a database with no
tables, because booting must not require this application's own database;
an uncached credential read on the same path needs exactly that guarantee
and nothing else, since resolve() can hand back a warm "configured" from
a database that has since stopped answering.

Both cache keys are bumped, as their comments require on a shape change.

Tests: five for the absence, two of them against the database cache store
read as the raw rows an operator would find in a dump, since phpunit.xml
runs the suite on the array store and the cache path was structurally
invisible -- which is why GoogleCloudStorageTest could assert that the private
key is not in the column while it sat in the cache. All five were run
against the unfixed appliers and fail there. The three "still configures
what it no longer caches" tests deliberately pass either way: they pin the
behaviour the fix must not break.
2026-08-28 23:46:19 +02:00

312 lines
11 KiB
PHP

<?php
declare(strict_types=1);
use App\Models\User;
use App\Modules\Files\Models\File;
use App\Modules\Files\Storage\ResolvingUploadDisk;
use App\Modules\Platform\Capabilities\Edition;
use App\Modules\Platform\Settings\ExternalStorageConfigApplier;
use App\Modules\Platform\Settings\ExternalStorageSettings;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Event;
use Illuminate\Support\Facades\Storage;
beforeEach(function () {
$this->admin = User::factory()->create();
});
test('the secret is encrypted at rest', function () {
ExternalStorageSettings::current()->fill(['secret' => 'super-secret-access-key'])->save();
$raw = DB::table('external_storage_settings')->value('secret');
expect($raw)->not->toBe('super-secret-access-key')
->and(ExternalStorageSettings::current()->secret)->toBe('super-secret-access-key');
});
test('the secret never reaches the cache store', function () {
// The column above is only half the promise: this class caches its
// resolved settings forever on every process boot, and a cache store
// encrypts nothing. See the same pair in MailProviderSettingsTest.
ExternalStorageSettings::current()->fill([
'active' => true,
'key' => 'AKIAEXAMPLE',
'secret' => 'super-secret-access-key',
'bucket' => 'my-bucket',
'region' => 'us-east-1',
])->save();
Cache::flush();
app(ExternalStorageConfigApplier::class)->apply();
$cached = Cache::get('platform.external_storage_settings.v3');
expect($cached)->toBeArray()
->and(json_encode($cached))->not->toContain('super-secret-access-key');
});
test('the secret never reaches the database cache store either', function () {
// Against the store config/cache.php actually defaults to, read as the
// raw row an operator would find in a dump.
config(['cache.default' => 'database']);
ExternalStorageSettings::current()->fill([
'active' => true,
'key' => 'AKIAEXAMPLE',
'secret' => 'super-secret-access-key',
'bucket' => 'my-bucket',
'region' => 'us-east-1',
])->save();
Cache::store('database')->flush();
app(ExternalStorageConfigApplier::class)->apply();
$rows = DB::table('cache')->pluck('value')->implode('|');
expect($rows)->not->toContain('super-secret-access-key');
});
test('apply() still configures the secret it no longer caches', function () {
// The other half: keeping the credential out of the cache must not
// stop the disk from being configured with it.
ExternalStorageSettings::current()->fill([
'active' => true,
'key' => 'AKIAEXAMPLE',
'secret' => 'super-secret-access-key',
'bucket' => 'my-bucket',
'region' => 'us-east-1',
])->save();
app(ExternalStorageConfigApplier::class)->flush();
app(ExternalStorageConfigApplier::class)->apply();
expect(config('filesystems.disks.files_external.secret'))->toBe('super-secret-access-key');
});
test('apply() is a no-op when not fully configured', function () {
$originalKey = config('filesystems.disks.files_external.key');
ExternalStorageSettings::current()->fill(['active' => true, 'key' => 'AKIA...'])->save();
app(ExternalStorageConfigApplier::class)->flush();
app(ExternalStorageConfigApplier::class)->apply();
expect(config('filesystems.disks.files_external.key'))->toBe($originalKey);
});
test('apply() overrides the files_external disk config once fully configured and active', function () {
ExternalStorageSettings::current()->fill([
'active' => true,
'key' => 'AKIAEXAMPLE',
'secret' => 'shh',
'bucket' => 'my-bucket',
'region' => 'us-east-1',
'endpoint' => 'https://minio.example.test',
'use_path_style' => true,
'root' => 'projectsend',
])->save();
app(ExternalStorageConfigApplier::class)->flush();
app(ExternalStorageConfigApplier::class)->apply();
expect(config('filesystems.disks.files_external.key'))->toBe('AKIAEXAMPLE')
->and(config('filesystems.disks.files_external.secret'))->toBe('shh')
->and(config('filesystems.disks.files_external.bucket'))->toBe('my-bucket')
->and(config('filesystems.disks.files_external.region'))->toBe('us-east-1')
->and(config('filesystems.disks.files_external.endpoint'))->toBe('https://minio.example.test')
->and(config('filesystems.disks.files_external.use_path_style_endpoint'))->toBeTrue()
->and(config('filesystems.disks.files_external.root'))->toBe('projectsend');
});
test('the ResolvingUploadDisk listener leaves new uploads on the local disk when not configured', function () {
app(ExternalStorageConfigApplier::class)->flush();
$event = new ResolvingUploadDisk($this->admin);
Event::dispatch($event);
expect($event->disk)->toBe('files');
});
test('the ResolvingUploadDisk listener redirects new uploads to files_external once configured and active', function () {
ExternalStorageSettings::current()->fill([
'active' => true,
'key' => 'AKIAEXAMPLE',
'secret' => 'shh',
'bucket' => 'my-bucket',
'region' => 'us-east-1',
])->save();
app(ExternalStorageConfigApplier::class)->flush();
$event = new ResolvingUploadDisk($this->admin);
Event::dispatch($event);
expect($event->disk)->toBe('files_external');
});
test('a real upload lands on the external disk once the backend is active, and local uploads made before activation are unaffected', function () {
Storage::fake('files');
Storage::fake('files_external');
uploadImageFile($this->admin);
$localFile = File::query()->latest('id')->first();
expect($localFile->disk)->toBe('files');
Storage::disk('files')->assertExists($localFile->path);
ExternalStorageSettings::current()->fill([
'active' => true,
'key' => 'AKIAEXAMPLE',
'secret' => 'shh',
'bucket' => 'my-bucket',
'region' => 'us-east-1',
])->save();
app(ExternalStorageConfigApplier::class)->flush();
uploadImageFile($this->admin);
$externalFile = File::query()->latest('id')->first();
expect($externalFile->id)->not->toBe($localFile->id)
->and($externalFile->disk)->toBe('files_external');
Storage::disk('files_external')->assertExists($externalFile->path);
// The first file, uploaded before activation, still resolves to local
// disk and is unaffected by the backend switch (no migration tool).
Storage::disk('files')->assertExists($localFile->refresh()->path);
expect($localFile->disk)->toBe('files');
});
test('staff can save external storage settings', function () {
config()->set('projectsend.edition', Edition::Community);
$this->actingAs($this->admin)->patch('/system/settings/storage', validStorageSettingsPayload([
'access_key' => 'AKIANEW',
'bucket' => 'renamed-bucket',
]))->assertRedirect();
$settings = ExternalStorageSettings::current();
expect($settings->key)->toBe('AKIANEW')
->and($settings->bucket)->toBe('renamed-bucket');
});
test('saving with a blank secret keeps the previously stored secret', function () {
$this->actingAs($this->admin)->patch('/system/settings/storage', validStorageSettingsPayload([
'secret' => 'first-secret',
]));
$this->actingAs($this->admin)->patch('/system/settings/storage', validStorageSettingsPayload([
'secret' => '',
'bucket' => 'renamed-bucket',
]));
$settings = ExternalStorageSettings::current();
expect($settings->secret)->toBe('first-secret')
->and($settings->bucket)->toBe('renamed-bucket');
});
test('saving external storage settings rejects invalid input', function () {
$this->actingAs($this->admin)->patch('/system/settings/storage', validStorageSettingsPayload([
'access_key' => '',
'bucket' => '',
'region' => '',
]))->assertSessionHasErrors(['access_key', 'bucket', 'region']);
});
test('saving external storage settings signals the queue worker to restart', function () {
Cache::forget('illuminate:queue:restart');
$this->actingAs($this->admin)->patch('/system/settings/storage', validStorageSettingsPayload());
expect(Cache::get('illuminate:queue:restart'))->not->toBeNull();
});
test('clients cannot save external storage settings', function () {
$this->actingAs(User::factory()->client()->create())
->patch('/system/settings/storage', validStorageSettingsPayload())
->assertForbidden();
});
test('the entire storage settings surface is unavailable in the cloud edition', function () {
config()->set('projectsend.edition', Edition::Cloud);
$this->actingAs($this->admin);
$this->get('/system/settings/storage')->assertNotFound();
$this->patch('/system/settings/storage', validStorageSettingsPayload())->assertNotFound();
});
test('cloud edition never honors a stored external storage backend, even one written outside the form', function () {
config()->set('projectsend.edition', Edition::Cloud);
// Simulate a stray/legacy row, bypassing the controller entirely —
// the runtime gate must hold regardless of how it got there.
ExternalStorageSettings::query()->create([
'active' => true,
'key' => 'AKIASTRAY',
'secret' => 'stray-secret',
'bucket' => 'stray-bucket',
'region' => 'us-east-1',
]);
app(ExternalStorageConfigApplier::class)->flush();
$event = new ResolvingUploadDisk($this->admin);
Event::dispatch($event);
expect($event->disk)->toBe('files');
});
test('a value cached while running as Community cannot leak into Cloud without an explicit flush', function () {
// Warm the cache while Community is active and the backend is fully
// configured — this is the exact scenario found manually: nothing
// besides saving the settings form ever calls flush(), so switching
// editions alone must not be enough to keep the old behavior alive.
config()->set('projectsend.edition', Edition::Community);
ExternalStorageSettings::current()->fill([
'active' => true,
'key' => 'AKIAEXAMPLE',
'secret' => 'shh',
'bucket' => 'my-bucket',
'region' => 'us-east-1',
])->save();
app(ExternalStorageConfigApplier::class)->flush();
app(ExternalStorageConfigApplier::class)->apply();
expect(config('filesystems.disks.files_external.bucket'))->toBe('my-bucket');
// Switch to Cloud — deliberately WITHOUT calling flush() again, since
// nothing in the app does that on an edition change either.
config()->set('projectsend.edition', Edition::Cloud);
config(['filesystems.disks.files_external.bucket' => '']);
app(ExternalStorageConfigApplier::class)->apply();
expect(config('filesystems.disks.files_external.bucket'))->toBe('');
$event = new ResolvingUploadDisk($this->admin);
Event::dispatch($event);
expect($event->disk)->toBe('files');
});
/**
* @param array<string, mixed> $overrides
* @return array<string, mixed>
*/
function validStorageSettingsPayload(array $overrides = []): array
{
return array_merge([
'active' => true,
'access_key' => 'AKIAEXAMPLE',
'secret' => 'shh',
'bucket' => 'my-bucket',
'region' => 'us-east-1',
'endpoint' => null,
'use_path_style' => false,
'root' => null,
], $overrides);
}