Remove hardcoded DEFINER clauses so migrations run as any DB user - #142
Open
drtechie wants to merge 1 commit into
Open
Remove hardcoded DEFINER clauses so migrations run as any DB user#142drtechie wants to merge 1 commit into
drtechie wants to merge 1 commit into
Conversation
Nine DEFINER clauses across eight migrations pin the object owner to a specific account. That makes a fresh deployment fail unless the database user happens to be named `piramaldev`, and V62 cannot be applied by any non-root account at all. dbiemr V13, V32, V34, V35, V71, V83 DEFINER=`piramaldev`@`%` dbiemr V62 DEFINER=`root`@`localhost` db1097identity V1 (x2) DEFINER=`root`@`localhost` MySQL only lets an account create an object whose DEFINER equals that account, unless it holds SET_USER_ID. Since 8.0.16 `root@localhost` is additionally a system account, so attributing an object to it also requires SYSTEM_USER — which is close to root-equivalent and not something an application account should hold. Measured on MySQL 8.0.46 as an ordinary unprivileged app user: DEFINER=`root`@`localhost` -> ERROR 1227, needs SYSTEM_USER DEFINER=`otherpartner`@`%` -> ERROR 1227, needs SET_USER_ID DEFINER=`piramaldev`@`%` -> OK, but only because that IS the account no DEFINER clause (this PR) -> OK with zero elevated privileges Removing the clause makes MySQL default the definer to CURRENT_USER, i.e. whichever account runs the migration. Verified: the resulting objects record definer = the migrating user. SQL SECURITY DEFINER is preserved everywhere it appeared, so the privilege semantics of each view and routine are unchanged — only the identity it binds to changes, from a hardcoded name to the actual owner. Editing applied migrations is safe here because FlywayMigrator.migrate() calls repair() on all four instances before migrate(), which realigns checksums in flyway_schema_history on every startup. Existing deployments are unaffected: these migrations have already been applied there and will not re-run. Beyond the create-time failure, a hardcoded DEFINER is a latent runtime fault: an object whose definer does not exist on the target server fails when queried with "The user specified as a definer does not exist", which surfaces as a broken screen rather than a failed deployment. Diff is 6 insertions and 9 deletions; three shapes were handled separately to keep each edit minimal — multi-line view headers, inline CREATE DEFINER=... PROCEDURE, and mysqldump's /*!50013 ... */ version-gated comments. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
💤 Files with no reviewable changes (3)
📝 WalkthroughWalkthroughThe migrations remove explicit MySQL ChangesSQL definer metadata updates
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
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
1 participant
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.
Nine
DEFINERclauses across eight migrations pin the object owner to a specific account. This makes a fresh deployment fail unless the database user happens to be namedpiramaldev, anddbiemr/V62cannot be applied by any non-root account at all.dbiemrV13, V32, V34, V35, V71, V83DEFINER=`piramaldev`@`%`dbiemrV62DEFINER=`root`@`localhost`db1097identityV1 (×2)DEFINER=`root`@`localhost`Why it breaks
MySQL only lets an account create an object whose
DEFINERequals that account, unless it holdsSET_USER_ID. Since 8.0.16root@localhostis additionally a system account, so attributing an object to it also requiresSYSTEM_USER— which is close to root-equivalent and not something an application account should hold.Because
FlywayMigratorruns in@PostConstruct, the failure presents asamrit-dbcrash-looping rather than as a clean migration error, which makes it expensive to diagnose.Measured on MySQL 8.0.46, as an ordinary unprivileged app user
DEFINER=`root`@`localhost`ERROR 1227— needsSYSTEM_USERDEFINER=`otherpartner`@`%`ERROR 1227— needsSET_USER_IDDEFINER=`piramaldev`@`%`DEFINERclause (this PR)The change
Removing the clause makes MySQL default the definer to
CURRENT_USER— whichever account runs the migration. Verified: the resulting objects recorddefiner = <migrating user>.SQL SECURITY DEFINERis preserved everywhere it appeared, so the privilege semantics of each view and routine are unchanged. Only the identity it binds to changes, from a hardcoded name to the actual owner.Diff is 6 insertions / 9 deletions. Three shapes were handled separately to keep each edit minimal:
CREATE/ALGORITHM/DEFINER/SQL SECURITY DEFINER)CREATE DEFINER=`u`@`h` PROCEDURE/*!50013 ... */version-gated commentsWhy this is safe for existing deployments
FlywayMigrator.migrate()calls.repair()on all four Flyway instances before.migrate(), which realigns checksums inflyway_schema_historyon every startup — so editing an applied migration does not cause a checksum mismatch. And on existing deployments these migrations have already been applied, so they will not re-run at all.Secondary benefit
A hardcoded
DEFINERis also a latent runtime fault: an object whose definer does not exist on the target server fails when queried withThe user specified as a definer does not exist. That surfaces as a broken screen rather than a failed deployment, long after the fact.How this was found
Standing up AMRIT MMU for a new partner on a clean Ubuntu 24.04 / MySQL 8.0 host with a per-partner database user. Without this change the deployment needs the DB user renamed to
piramaldevand a temporarySYSTEM_USERgrant during migration; with it, neither is required.🤖 Generated with Claude Code
Summary by CodeRabbit