yogthos / migratus

MIGRATE ALL THE THINGS!
644 stars 95 forks source link

Sanitation of migration-table-name breaks existing migrations #230

Closed NoahTheDuke closed 1 year ago

NoahTheDuke commented 1 year ago
$ lein --version
Leiningen 2.9.8 on Java 11.0.16 OpenJDK 64-Bit Server VM
$ clojure --version
Clojure CLI version 1.11.1.1155
$ psql --version
psql (PostgreSQL) 14.5 (Ubuntu 14.5-0ubuntu0.22.04.1)

My :migration-table-name is "public.migratus". After updating from 1.3.3 to 1.4.4, migratus stripped it to be publicmigratus. This is a pretty big change and feels like it should have been highlighted in the changelog. If this change is permanent, can a new flag be added to not sanitize the table name?

https://github.com/yogthos/migratus/blob/fbb27047b0bae88af406e6c5a5eca565d653652b/src/migratus/database.clj#L37

Originating PR

ryanechternacht commented 1 year ago

Yeah... this is a BIG DEAL for us.

We ran into the exact problem described above

  1. we were writing to public.migratus, which got sanitized down to publicmigratus which is a different table
  2. We ran a migration, which failed on step 1 (it couldn't find the db)
  3. Migratus tried to rollback our first migration (since the first migration failed)
  4. The first migrations created schemas, so the first rollback deleted them drop schema public cascade
  5. WE FUCKING DELETED A DATABASE

I can't stress enough, that this was a MASSIVE change, masquerading as a small upgrade.

Also, I'd be curious to understand the attack vector this is designed to prevent. If I have access to supply the migration config, I would assume I have a lot of access. Is there really a unique attack vector here worth preventing?

Afterword: We've taken the step of having our first rollback NOT delete stuff, to prevent this issue going forward. But also, we can't upgrade to the latest version, since you won't let us use the table name we prefer. So as @NoahTheDuke mentioned, I think there needs to be a work around here (or we'll be pegged on our current version forever 😢 )

ryanechternacht commented 1 year ago

Thank you!!!!!