Skip to content

fix: an install rollback drops only the database it created - #926

Merged
blaipr merged 1 commit into
mainfrom
fix/an-install-rollback-drops-only-what-it-created
Sep 17, 2026
Merged

blaipr merged 1 commit into
mainfrom
fix/an-install-rollback-drops-only-what-it-created

Conversation

@blaipr

@blaipr blaipr commented Sep 17, 2026

Copy link
Copy Markdown
Member

MysqlSetup::rollback() ended a failed install with an unconditional

DROP DATABASE IF EXISTS `<dbname>`

and nothing established whose database that was.

install/install is unauthenticated by necessity. Installer::install() calls
checkDatabaseAvailability() before anything is created and createDatabase() well after it, with
no lock in between — so two requests both pass the availability check, the second fails on
CREATE SCHEMA because the name is now taken, lands in the catch, and drops the database the
first had just finished installing into. config.xml already said installed=1, so the instance
claimed to be installed with no schema behind it and every later request went to
error/databaseError.

Two near-simultaneous requests is all it takes. No attacker is required — an impatient double-click
on the install button does it.

The fix is already written, one field over

The same method drops the runtime user only when $dbUser names one — that is, only when this
run created it. The database had no such flag. It does now: Installer sets $createdDatabase the
moment createDatabase() returns, and the drop is gated on it.

The hazard was understood when this was written; the comment above checkDatabaseAvailability()
says so —

Validate the target before anything is created: a failure here must not trigger a rollback, which
could otherwise touch pre-existing data

— it was enforced at the check rather than at the rollback, and only the check is racy.

What this does and does not fix

It removes the destructive outcome: the losing request now fails and drops nothing, and the winner's
install survives. It does not serialise concurrent installs — the loser still gets an error
about the schema already existing, which is the correct answer to what it asked. A lock across the
whole install would be a separate change; this one makes losing the race harmless.

Hosting mode is untouched: it takes the other branch, dropping the named tables and views rather
than a database, and createDatabase() is a documented no-op there.

Tests

Four, two new and two rewritten:

  • a rollback triggered before createDatabase() returns drops no database — the losing
    request's exact shape;
  • a rollback after it does drop it, or a genuinely failed install would leave its own half-built
    schema and refuse every retry;
  • MySQLTest's two existing non-hosting rollback tests now say createdDatabase: true, since that
    is the case they were always describing;
  • and a new one asserting exec() is never reached when the flag is false.

Mutation-verified: reverting src/ fails both of the new InstallerTest cases.

Worth noting for whoever writes the next test here — the existing rollback tests all set
hostingMode(true), which skips setupDbUser(). A non-hosting test has to stub it, or the
destructuring at Installer.php:226 raises "Undefined array key 0" and the test passes with a
warning rather than failing.

MysqlSetup::rollback() ended a failed install with an unconditional DROP DATABASE, and
nothing established whose database it was. install/install is unauthenticated by necessity,
and Installer::install() calls checkDatabaseAvailability() before anything is created and
createDatabase() well after it, with no lock between — so two requests both pass the
availability check, the second fails on CREATE SCHEMA because the name is taken, and its
rollback dropped the database the first had just finished installing into. config.xml
already said installed=1, so the instance claimed to be installed with no schema behind it.

Two near-simultaneous requests is all it takes; an impatient double-click on the install
button will do it.

The same method already drops the runtime user only when $dbUser names one — only when this
run created it. The database had no such flag, and now does: Installer sets it the moment
createDatabase() returns, and the drop is gated on it.

The hazard was understood when this was written — the comment above
checkDatabaseAvailability() says a failure there must not trigger a rollback that could
touch pre-existing data. It was enforced at the check rather than at the rollback, and only
the check is racy.

This does not serialise concurrent installs: the loser still fails, correctly, on the schema
already existing. It makes losing the race harmless rather than destructive.
@blaipr
blaipr merged commit 9cd4b5e into main Sep 17, 2026
8 checks passed
@blaipr
blaipr deleted the fix/an-install-rollback-drops-only-what-it-created branch September 17, 2026 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant