Skip to content

fix: skip app-user mysql.user probe when app user matches master/migrator - #8664

Merged
CarolineDenis merged 4 commits into
specify:mainfrom
idabblewith:fix/app-user-mysql-user-probe-guard
Oct 20, 2026
Merged

CarolineDenis merged 4 commits into
specify:mainfrom
idabblewith:fix/app-user-mysql-user-probe-guard

Conversation

@idabblewith

@idabblewith idabblewith commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #8305

Problem

On startup, sp7_db_setup_check.sh runs an app-user existence probe
(SELECT COUNT(*) FROM mysql.user ...) BEFORE the SAME_MASTER_AND_APP /
SAME_MIGRATOR_AND_APP guard. Under set -euo pipefail this probe runs
even when the app user is identical to the master/migrator user.

On managed MySQL (e.g. Azure Database for MySQL) the admin account is
not root and is denied access to the mysql.* system tables, so the probe
fails with ERROR 1142 ... SELECT command denied ... for table 'user'.
Since 7.12.0.7 the entrypoint treats a setup failure as fatal
("Database setup failed; exiting."), so the container CrashLoops and
never starts — even though the app user needs no setup (it reuses
master's privileges). The existing # BUG: comment already noted this
case should be skipped. Regressed in #8156; 7.12.0.6 and earlier are
unaffected.

Fix

Move the app-user probe/create block inside the else branch of the
existing SAME_MASTER_AND_APP / SAME_MIGRATOR_AND_APP guard, so it only
runs when the app user is genuinely separate — matching the migrator
block directly above. No query or privilege logic changed; the block is
only relocated into its guard.

Testing instructions

  1. Set MASTER_NAME = MIGRATOR_NAME = APP_USER_NAME against a MySQL where
    the account cannot read mysql.user (e.g. Azure Database for MySQL).
  2. Before: container exits with ERROR 1142 ... for table 'user'.
  3. After: setup logs "App user ... uses the same credentials as master.
    Skipping..." and startup proceeds.
  4. Regression: with a distinct app user, create/grant/verify still runs.

Summary by CodeRabbit

  • Chores
    • Updated database setup to reuse master or migrator accounts when app credentials match, avoiding separate account creation and grants. Separate app-user setup remains for distinct credentials.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9e308f99-d150-4e3a-ab0d-6d5778fdd06a
📥 Commits

Reviewing files that changed from the base of the PR and between 1956fd3 and 6820cef.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 58ab5d70-3dba-40ea-8ddb-8aed80e0b01b
📥 Commits

Reviewing files that changed from the base of the PR and between 41ee340 and 957f484.

📒 Files selected for processing (1)
  • sp7_db_setup_check.sh

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The app-user setup now checks whether its credentials match the master or migrator account. If they match, the script skips separate account setup and reports the privileges used. Otherwise, it proceeds with separate app-user setup.

Changes: App-user setup

Layer / File(s) Summary
App-user identity branch
sp7_db_setup_check.sh
When app credentials match the master or migrator credentials, the script skips separate account setup and reports reliance on that account’s privileges. When neither matches, it proceeds with separate app-user setup.

Suggested reviewers: melton-jason

Priority: ➖ Normal

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 957f4

The change avoids querying the user table when the app user reuses the master or migrator credentials, which should prevent startup failures on managed MySQL. No merge-blocking risk was identified.

Pre-merge checks: Passed 4 · Failed 2

Failed Failed checks (2 warnings)

Check name Status Explanation Resolution
Automatic Tests Warning The PR changes startup database behavior but adds no automatic test. The diff contains only sp7_db_setup_check.sh. docker-entrypoint.sh treats a setup failure as fatal, and the existing backend workfl… Add an automated test that exercises sp7_db_setup_check.sh with mocked or containerized MariaDB commands. Assert that matching master/app and migrator/app credentials skip the mysql.user probe, and that distinct app credentials still execut…
Testing Instructions Warning The instructions are clear for the all-three-identities-equal case and for a distinct app user. However, the changed script has separate SAME_MASTER_AND_APP and SAME_MIGRATOR_AND_APP branches. The… Add a separate scenario with MASTER_NAME different from MIGRATOR_NAME, while APP_USER_NAME and MIGRATOR_NAME use the same credentials. Verify the log reports that the app user uses the migrator credentials, the app-user mysql.user…
Passed Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: skipping the MySQL user probe when the app user matches the master or migrator credentials.
Linked Issues check Passed Issue [#8305] is closed and supplies historical context only. No active directly linked issue remains, so this pull request has no linked-issue coding requirements to satisfy.
Out of Scope Changes check Passed The diff changes only sp7_db_setup_check.sh. It places the existing app-user probe, creation, grant, and verification flow in the separate-user branch. It skips that flow when the app user matches the…
Full details: Automatic Tests

Explanation

The PR changes startup database behavior but adds no automatic test. The diff contains only sp7_db_setup_check.sh. docker-entrypoint.sh treats a setup failure as fatal, and the existing backend workflow does not invoke this setup script. Manual testing instructions do not protect the matching-credentials regression or the separate-app-user path.

Resolution

Add an automated test that exercises sp7_db_setup_check.sh with mocked or containerized MariaDB commands. Assert that matching master/app and migrator/app credentials skip the mysql.user probe, and that distinct app credentials still execute creation, grants, and verification. Run the test in CI.

Full details: Testing Instructions

Explanation

The instructions are clear for the all-three-identities-equal case and for a distinct app user. However, the changed script has separate SAME_MASTER_AND_APP and SAME_MIGRATOR_AND_APP branches. The proposed matching test sets MASTER_NAME = MIGRATOR_NAME = APP_USER_NAME, so it only exercises the master branch; it does not verify the app-user-equals-migrator branch.

Resolution

Add a separate scenario with MASTER_NAME different from MIGRATOR_NAME, while APP_USER_NAME and MIGRATOR_NAME use the same credentials. Verify the log reports that the app user uses the migrator credentials, the app-user mysql.user probe does not run, and app-user creation, grant, and verification are skipped. Retain the distinct-app-user regression scenario.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@CarolineDenis
CarolineDenis merged commit 39db812 into specify:main Oct 20, 2026
11 of 13 checks passed
@github-project-automation github-project-automation Bot moved this from 📋Back Log to ✅Done in General Tester Board Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅Done

Development

Successfully merging this pull request may close these issues.

Cannot setup a new Specify 7 database locally / cannot access Specify 7 databases not opened in v7.12 locally

4 participants