[2.x] Harden the updater: authenticate it, and pause the queue while it runs - #4930
Merged
Conversation
The updater is served while the code is ahead of the database — between new files landing and their migrations running. Two things were wrong with that window. Authentication. The updater ran pending migrations for anyone who submitted the right database password, but it only checked the password when one was set. An install with no database password — which the installer produces, and which is valid for socket-auth MySQL, MariaDB and Postgres — had no check at all, so a bodyless POST ran every pending migration. The username and password are now both verified for those drivers, so a passwordless install is still gated by the username the installer always requires. SQLite has neither, so it falls back to the database file's name. The comparison uses hash_equals, and every failure returns the same generic error so it cannot be used as an oracle. The queue. Nothing stopped a worker running jobs against a schema that was mid-migration. The queue is now paused while migrations run and resumed when they finish. The web updater arms the pause the moment it first serves the update page — the danger starts when the version drifts, not when an admin clicks Update. `php flarum migrate`, run at any time from the CLI, pauses around its own run too, but only when there is work to do, so a routine no-op migrate does not idle every worker. Resume always runs in a finally, and unconditionally, so a failed migration — or a no-op run reached through the updater after the web arm already paused — never leaves the queue stuck. This uses the existing queue:pause/queue:resume flag, not maintenance mode, so the forum stays online to visitors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #0000
Changes proposed in this pull request:
Hardens the updater — the page shown while the code is ahead of the database, between uploading new files and running migrations. Two things were wrong with that window.
Authentication. The updater ran pending migrations for anyone who submitted the right
database.password, but it only checked the password when one was set. An install with no database password — which the installer produces, and which is valid for socket-auth MySQL/MariaDB/Postgres — had no check at all: a bodyless POST ran every pending migration. Now the username and password are both verified for those drivers, so a passwordless install is still gated by the username (which the installer always requires). SQLite has neither, so it falls back to the database file's name. The comparison useshash_equals, and every failure returns the same generic error.The queue. Nothing stopped a queue worker running jobs against a schema that was mid-migration. The queue is now paused while migrations run and resumed when they finish. The web updater arms the pause the moment it first serves the update page — the danger starts when the version drifts, not when an admin clicks Update.
php flarum migrate(run any time from the CLI) pauses around its own run too, but only when something is actually pending, so a routine no-opmigratedoesn't idle every worker. Resume always runs in afinally, so a failed migration never leaves the queue stuck. This uses the existingqueue:pause/queue:resumemechanism — not maintenance mode, so the forum stays online to visitors.Impacted:
UpdateController,IndexControllerand the updater form;MigrateCommand;InstalledApp.Reviewers should focus on:
flarum.sqliteis guessable. The case for it: SQLite is a supported production driver, and leaving one driver with zero check while the others have one is hard to defend. It raises the bar from nothing to "must know the db name", no more. Theroot+ no-password case stays guessable for the same reason it always was — indefensible from HTTP, and refusing the web updater would break the shared-hosting users it exists for.MigrateCommandmust resume unconditionally to clear it — otherwise a no-op run reached through the updater leaves the queue paused forever. This exact case stuck the queue on my dev forum during development; there's a test for it now.Screenshot
Necessity
Confirmed
composer test).New tests:
UpdateControllerTestandIndexControllerTest(unit — verification across all drivers, oracle-free errors, and the form asking for exactly what's checked);MigrateCommandQueuePauseTestand additions toUpgradePageTest(integration — pause/resume, no-op leaves the queue alone, resume-on-failure, the web arm, and the no-op-clears-a-prior-pause regression). All load-bearing behaviours mutation-tested.Required changes: