Read settings written before the columns they name existed

Reported by @apps3000 in #1770. Upgrading a container from 2.0 or 2.1
with external storage configured restart-loops, and says the database is
unreachable while the database is fine.

A row hydrated from the database does not get the model's column
defaults — only a new model does. So a row written before
external_storage_settings.provider existed reads that column as null,
and the enum match in isConfigured() throws UnhandledMatchError.

That would be a small bug anywhere else. It is not here, because
PlatformServiceProvider::boot() reads these settings on every process
boot, and boot happens before `artisan migrate` runs. During an upgrade
the code is new and the schema is still old, so every artisan command in
that window dies — including `projectsend:update`, the one that would
have added the column. Reordering the entrypoint or using a lighter
readiness probe does not help for that reason; the crash is in the
bootstrap, not in the probe.

current() now applies the model's declared defaults to any column the
hydrated row does not have. That closes the window for every column with
a default rather than for the one where it was found, and goes inert the
moment the schema is current. The match in isConfigured() is left total
on purpose: a default arm would swallow a real unhandled case, and the
invariant it needs now holds at the one place the row is read.

The probe's message is the other half. It boots the whole application,
so it fails both when the database is absent and when the application
cannot start, and it reported the second as the first — sending an
operator off checking credentials that were never wrong. It now prints
the error it actually hit and says which of the two it looks like.

Verified end to end against a 2.1-shaped database: `artisan migrate`
dies with UnhandledMatchError before the change and completes after it,
leaving the row reading as S3 with its bucket intact.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QmyH342d8MuW3pDuE9mbtS

app/Modules/Platform/Settings/ExternalStorageSettings.php

@@ -75,7 +75,38 @@ class ExternalStorageSettings extends Model
 
     public static function current(): self
     {
-        return static::query()->firstOrNew([]);
+        $settings = static::query()->firstOrNew([]);
+
+        // Column defaults — the $attributes array above — apply to a NEW
+        // model, never to one hydrated from a row. So a row written by an
+        // older release, before one of these columns existed, reads that
+        // column as null however sensible its default is.
+        //
+        // That matters here more than it would anywhere else, because
+        // PlatformServiceProvider::boot() reads these settings on every
+        // process boot — and boot happens BEFORE `artisan migrate` runs.
+        // For the length of an upgrade the code is new and the schema is
+        // still old, and every artisan command in that window, including
+        // the one that would run the migrations, boots through here.
+        //
+        // A null `provider` made the match in isConfigured() throw
+        // UnhandledMatchError, which the official image's readiness probe
+        // reported to the operator as "database unreachable" — on a
+        // perfectly reachable database, in a container that then
+        // restart-looped without ever reaching the migration that would
+        // have fixed it (#1770, upgrading from 2.0/2.1 with external
+        // storage configured).
+        //
+        // Applying the defaults to a hydrated row closes that window for
+        // every column that has one, rather than for the single column
+        // where it was found. Inert on any install whose schema is current.
+        foreach ((new self)->getAttributes() as $column => $default) {
+            if (! array_key_exists($column, $settings->getAttributes())) {
+                $settings->setAttribute($column, $default);
+            }
+        }
+
+        return $settings;
     }
 
     /**
@@ -93,6 +124,11 @@ class ExternalStorageSettings extends Model
         // authenticate with different things entirely: S3 wants a key and
         // a secret, GCS wants a service account key file.
         //
+        // The match is deliberately left total rather than given a default
+        // arm: current() guarantees a provider even on a row older than the
+        // column, and a default arm here would quietly swallow a genuinely
+        // unhandled case instead of naming it.
+        //
         // Unless S3 is being asked to authenticate as the machine it is
         // running on, in which case there is no credential to fill in at
         // all and demanding one would leave the disk permanently

docker/production/entrypoint.sh

@@ -58,12 +58,26 @@ fi
 # Wait for the database. `migrate` against a database still starting up is
 # the single most common first-run failure, and a bare failure here would
 # restart-loop the container with a stack trace instead of a clear message.
+#
+# The probe boots the whole application, so it fails for two quite different
+# reasons: the database really is not there yet, or it is there and the
+# application could not start. Both used to be reported as the first one,
+# which sent an operator off checking credentials that were never wrong
+# (#1770). The last failure is kept and printed, so whichever it was is on
+# screen instead of being guessed at.
 if [ "$1" = "supervisord" ] || [ "$1" = "/usr/bin/supervisord" ]; then
     i=0
-    until su-exec www-data php artisan db:show --quiet >/dev/null 2>&1; do
+    until probe_error=$(su-exec www-data php artisan db:show --quiet 2>&1); do
         i=$((i + 1))
         if [ "$i" -ge 60 ]; then
-            echo "projectsend: database unreachable after 60s — check DB_HOST, DB_DATABASE and credentials" >&2
+            echo "projectsend: gave up waiting for the database after 60s." >&2
+            echo "projectsend: the last attempt failed with:" >&2
+            printf '%s\n' "$probe_error" | tail -n 20 >&2
+            echo "projectsend:" >&2
+            echo "projectsend: if that names the database host, the connection or the credentials," >&2
+            echo "projectsend: check DB_HOST, DB_DATABASE, DB_USERNAME and DB_PASSWORD." >&2
+            echo "projectsend: if it is an application error, the database is fine and this is a" >&2
+            echo "projectsend: bug — please report it at https://github.com/projectsend/projectsend/issues" >&2
             exit 1
         fi
         [ "$i" = 1 ] && echo "projectsend: waiting for the database..."

tests/Feature/Platform/ExternalStorageSettingsTest.php

@@ -8,9 +8,12 @@ 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\Database\Schema\Blueprint;
 use Illuminate\Support\Facades\Cache;
+use Illuminate\Support\Facades\Crypt;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\Event;
+use Illuminate\Support\Facades\Schema;
 use Illuminate\Support\Facades\Storage;
 
 beforeEach(function () {
@@ -239,6 +242,43 @@ test('new uploads go to the external disk with no stored credentials when the se
     expect($event->disk)->toBe('files_external');
 });
 
+test('booting against a row older than the provider column does not throw', function () {
+    // #1770. The exact shape of a 2.0/2.1 install with S3 configured,
+    // upgrading to 2.2 or later: the code is new, the schema is still old,
+    // and boot happens before `artisan migrate` gets a chance to run.
+    //
+    // The failure this pins was not a wrong answer but a fatal one — an
+    // UnhandledMatchError on a null provider, thrown during
+    // PlatformServiceProvider::boot(), which took down every artisan
+    // command including the one that would have added the column. The
+    // official image reported it as "database unreachable" and
+    // restart-looped.
+    legacyStorageRow();
+
+    app(ExternalStorageConfigApplier::class)->flush();
+    app(ExternalStorageConfigApplier::class)->apply();
+
+    // Read as S3, which is the only thing a row predating the column could
+    // have been — and is what the migration's own default says.
+    expect(config('filesystems.disks.files_external.bucket'))->toBe('my-bucket')
+        ->and(config('filesystems.disks.files_external.key'))->toBe('AKIAEXAMPLE')
+        ->and(config('filesystems.disks.files_external.driver'))->toBe('s3');
+});
+
+test('an upload still reaches the external disk while the schema is mid-upgrade', function () {
+    // The other half: not throwing is not the same as behaving. An
+    // installation that was storing files in a bucket before the upgrade
+    // must not quietly start writing them to local disk during it.
+    legacyStorageRow();
+
+    app(ExternalStorageConfigApplier::class)->flush();
+
+    $event = new ResolvingUploadDisk($this->admin);
+    Event::dispatch($event);
+
+    expect($event->disk)->toBe('files_external');
+});
+
 test('staff can save external storage settings', function () {
     config()->set('projectsend.edition', Edition::Community);
 
@@ -417,6 +457,29 @@ test('a value cached while running as Community cannot leak into Cloud without a
     expect($event->disk)->toBe('files');
 });
 
+/**
+ * An external storage row exactly as 2.0/2.1 wrote it: S3 credentials in
+ * place, and none of the columns added since.
+ *
+ * Written with the query builder and then stripped back, rather than
+ * hand-rolled SQL, so it stays honest if the table changes shape again.
+ */
+function legacyStorageRow(): void
+{
+    DB::table('external_storage_settings')->insert([
+        'active' => true,
+        'key' => 'AKIAEXAMPLE',
+        'secret' => Crypt::encryptString('shh'),
+        'bucket' => 'my-bucket',
+        'region' => 'us-east-1',
+        'use_path_style' => false,
+    ]);
+
+    Schema::table('external_storage_settings', function (Blueprint $table) {
+        $table->dropColumn(['provider', 'key_file', 'use_instance_role']);
+    });
+}
+
 /**
  * @param  array<string, mixed>  $overrides
  * @return array<string, mixed>