Merge pull request #6389 from specify/issue-6121#8149
Open
CarolineDenis wants to merge 9 commits into
Open
Conversation
Automatic DB user creation for new Specify 7 instances
📝 WalkthroughWalkthroughThis PR implements a three-tier database user architecture (master, migrator, app) with environment-driven credential defaults, a startup orchestration script that creates/validates users and privileges, expanded Django multi-alias DATABASES with a DB_ALIAS selector, and migration tooling/entrypoint changes to run targeted migrations. ChangesDatabase User Setup and Runtime Integration
🚥 Pre-merge checks | ❌ 2❌ Failed checks (2 warnings)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
Author
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
specifyweb/settings/__init__.py (1)
90-100:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStop building the SQLAlchemy URLs with master credentials.
SA_DATABASE_URLandSA_TEST_DB_URLstill always useMASTER_NAME/MASTER_PASSWORD, so any SQLAlchemy code path bypasses the newapp/migrationssplit and continues to depend on elevated credentials. That undercuts the least-privilege model even whenDATABASES["default"]is routed correctly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specifyweb/settings/__init__.py` around lines 90 - 100, get_sa_db_url currently hard-codes MASTER_NAME/MASTER_PASSWORD so SA_DATABASE_URL and SA_TEST_DB_URL keep using elevated creds; change the construction so SA_DATABASE_URL and SA_TEST_DB_URL use the credentials from the Django DATABASES["default"] entry instead of MASTER_NAME/MASTER_PASSWORD. Update get_sa_db_url to accept explicit user/password (or build the URL inline) and use DATABASES["default"]["USER"], DATABASES["default"]["PASSWORD"], DATABASES["default"]["HOST"], DATABASES["default"]["PORT"] when setting SA_DATABASE_URL and SA_TEST_DB_URL (keeping the same db name and charset and preserving the f'test_{DATABASE_NAME}' behavior).
🧹 Nitpick comments (2)
Dockerfile (1)
18-18: 💤 Low valueRedundant
mariadb-clientinstallation.
mariadb-clientis installed at line 18 in thecommonstage, butrun-common(which inheritsFROM common) reinstalls it at line 141. The second installation is unnecessary and adds build time.♻️ Suggested fix at line 141
- apt-get -y install --no-install-recommends rsync mariadb-client && \ + apt-get -y install --no-install-recommends rsync && \Also applies to: 141-141
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` at line 18, The Dockerfile installs mariadb-client twice: once in the common stage and again in the run-common stage; remove the redundant installation from the run-common stage so mariadb-client is only installed in the common stage. Locate the two stages named "common" and "run-common" in the Dockerfile, and delete the mariadb-client package entry from the apt-get install list in the run-common stage (leaving the installation in the common stage intact); verify no other dependent lines in run-common assume a separate install step..github/workflows/test.yml (1)
67-72: ⚡ Quick winUse distinct CI users so this workflow actually exercises the new permission split.
Right now
master,migrator, andappall resolve to the same credentials, so the test job won't catch permission-boundary regressions in the new setup script or alias wiring.Also applies to: 125-130
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test.yml around lines 67 - 72, The workflow currently uses identical credentials for MASTER, MIGRATOR and APP users so tests won't validate permission boundaries; update the environment variables (MASTER_NAME / MASTER_PASSWORD, MIGRATOR_NAME / MIGRATOR_PASSWORD, APP_USER_NAME / APP_USER_PASSWORD) to use distinct usernames and passwords (and mirror the same distinct values in the second block referenced around lines 125-130) so CI exercises the different permission sets and reveals regressions in the setup/alias wiring.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker-entrypoint.sh`:
- Line 13: Fix the typo in the inline comment that reads "Setup db users and run
mirgations" by updating it to "Setup db users and run migrations" (locate the
comment adjacent to the ./sp7_db_setup_check.sh invocation in
docker-entrypoint.sh and replace "mirgations" with "migrations").
In `@sp7_db_setup_check.sh`:
- Around line 40-43: The required-vars check omits the master DB credentials
used later; update the validation in sp7_db_setup_check.sh to include
MASTER_USER_NAME and MASTER_USER_PASSWORD so the script fails early with a clear
error if those environment variables are missing; specifically modify the
conditional that checks DB_HOST, DB_PORT, MIGRATOR_PASSWORD, DB_NAME,
APP_USER_NAME, and APP_USER_PASSWORD to also require MASTER_USER_NAME and
MASTER_USER_PASSWORD and ensure the echo error message mentions the missing
master credentials.
- Around line 76-95: The shell inserts unescaped env vars (DB_NAME,
MIGRATOR_NAME, MIGRATOR_PASSWORD, MASTER_USER_PASSWORD, etc.) directly into
mysql SQL strings (see DB_EXISTS and USER_EXISTS checks and the CREATE DATABASE
/ CREATE USER mysql -e invocations), which allows quotes/backticks in secrets to
break or alter SQL; fix by adding small helper functions to safely escape
identifiers and string literals before interpolation (e.g., escape SQL
identifiers by doubling/backslash-escaping backticks and wrap in backticks, and
escape SQL string literals by replacing single-quote with '\'' and wrapping in
single quotes), then use those helpers when building the queries for DB_NAME,
MIGRATOR_NAME, MIGRATOR_USER_HOST, MIGRATOR_PASSWORD, and any other env var used
in mysql -e calls so all CREATE DATABASE/CREATE USER and SELECT queries use
escaped values.
- Around line 1-5: Enable fail-fast behavior so the script aborts if the initial
migration fails: add strict shell options (e.g., set -euo pipefail) near the
top, and/or explicitly check the exit status of the base_specify_migration
invocation and exit with non‑zero if it fails before running the next migration
step (the invocation of manage.py migrate or any "second migration" commands);
ensure any conditional gating uses the exit code of the base_specify_migration
command (or the variable/command name base_specify_migration) to prevent
continuing on partial initialization.
- Around line 129-152: The current loop (reading GRANTS_PARSED and setting
migrator_has_access) treats any grant other than USAGE as sufficient; change the
privilege check so migrator_has_access is only true when the parsed privs string
contains migration-capable privileges (e.g. ALL PRIVILEGES or at least one of
CREATE, ALTER, DROP, INDEX, CREATE VIEW, CREATE ROUTINE or other DDL privileges
your app requires) rather than any non-USAGE token. Update the condition that
sets migrator_has_access (the block that computes privs via sed and currently
uses grep -Eiq '(^|[, ])USAGE([, ]|$)') to instead match ALL or a regex like
CREATE|ALTER|DROP|INDEX|CREATE[[:space:]]VIEW|CREATE[[:space:]]ROUTINE
(case-insensitive) and only set migrator_has_access=true when that match
succeeds; keep the existing fallback branch that reassigns
MIGRATOR_NAME/MIGRATOR_PASSWORD/MIGRATION_DB_ALIAS unchanged.
- Around line 195-239: The verifier currently expects a single GRANT line to
include all REQUIRED_PRIVS; instead, build a union of privileges from all
relevant grant lines before deciding. Modify the loop over APP_GRANTS_LINES to,
for each line that matches a relevant scope (global ON *.* TO or DB-scoped ON
`?${DB_NAME}`?.* TO), extract the privilege list portion (text between "GRANT"
and "ON"), normalize commas/spaces, and add each privilege into a cumulative set
(associative array or deduped list); after processing all lines, check that
every REQUIRED_PRIVS member is present in that union and set
app_has_required_permissions accordingly (rather than calling
has_all_privs_in_line per-line). Keep APP_GRANTS_RAW, REQUIRED_PRIVS,
APP_GRANTS_LINES, DB_NAME, app_has_required_permissions and the final fallback
logic intact.
- Around line 91-95: The condition guarding the CREATE USER branch is checking
APP_USER_NAME but the branch creates MIGRATOR_NAME; change the guard to check
MIGRATOR_NAME instead (i.e., use MIGRATOR_NAME != "root") so we verify the
migrator account before attempting CREATE USER; keep the existing USER_EXISTS
check and CREATE USER call (MIGRATOR_NAME, MIGRATOR_USER_HOST,
MIGRATOR_PASSWORD) unchanged so the root fallback still works.
In `@specifyweb/settings/__init__.py`:
- Around line 85-88: Validate the DJANGO_DB_ALIAS before using it to index
DATABASES: check that DB_ALIAS (from os.getenv("DJANGO_DB_ALIAS", "default")) is
a key in the DATABASES dict and if not either set DB_ALIAS = "default" or raise
a clear ImportError/RuntimeError with a descriptive message; update the block
around DB_ALIAS and DATABASES so the code only does
deepcopy(DATABASES[DB_ALIAS]) when DB_ALIAS is present, otherwise fallback to
default or raise an explicit error indicating the invalid alias and listing
valid keys.
In `@specifyweb/settings/specify_settings.py`:
- Around line 62-72: Replace the hardcoded placeholders for MIGRATOR_NAME,
MIGRATOR_PASSWORD, APP_USER_NAME, and APP_USER_PASSWORD with environment lookups
that fall back to the intended defaults (use os.environ.get), e.g. derive
MASTER_NAME and MASTER_PASSWORD from env first, then set MIGRATOR_* =
os.environ.get('MIGRATOR_NAME', MASTER_NAME) and MIGRATOR_PASSWORD =
os.environ.get('MIGRATOR_PASSWORD', MASTER_PASSWORD), and similarly
APP_USER_NAME = os.environ.get('APP_USER_NAME', MIGRATOR_NAME) and
APP_USER_PASSWORD = os.environ.get('APP_USER_PASSWORD', MIGRATOR_PASSWORD); also
fix the existing fallback bug by using the correct env key ('MASTER_PASSWORD')
when computing MASTER_PASSWORD instead of the duplicated wrong key.
In `@specifyweb/specify/management/commands/base_specify_migration.py`:
- Around line 33-34: The bare except that sets alias = 'master' should be
replaced with a narrow exception handler: catch the specific exceptions that can
occur when computing alias (e.g., IndexError, KeyError, ValueError) and
optionally log the exception before falling back to 'master' (e.g., except
(IndexError, KeyError, ValueError) as e: logger.warning("... %s", e); alias =
'master'). Do the same replacement for the second occurrence at lines 46-47 so
both bare except blocks are removed in favor of specific exception handling and
logging.
- Around line 31-35: The fallback to 'master' is a no-op because
transaction.atomic(using=alias) doesn't raise until entered; change the check to
actively open the connection and propagate the proper alias: attempt to access
connections[alias] and call conn.ensure_connection() (or conn.cursor()) inside
the try/except; on exception set alias = 'master' and reassign conn =
connections[alias] before using transaction.atomic(using=alias); update usage of
transaction.atomic, conn, connections and alias so they always refer to the same
resolved alias.
---
Outside diff comments:
In `@specifyweb/settings/__init__.py`:
- Around line 90-100: get_sa_db_url currently hard-codes
MASTER_NAME/MASTER_PASSWORD so SA_DATABASE_URL and SA_TEST_DB_URL keep using
elevated creds; change the construction so SA_DATABASE_URL and SA_TEST_DB_URL
use the credentials from the Django DATABASES["default"] entry instead of
MASTER_NAME/MASTER_PASSWORD. Update get_sa_db_url to accept explicit
user/password (or build the URL inline) and use DATABASES["default"]["USER"],
DATABASES["default"]["PASSWORD"], DATABASES["default"]["HOST"],
DATABASES["default"]["PORT"] when setting SA_DATABASE_URL and SA_TEST_DB_URL
(keeping the same db name and charset and preserving the f'test_{DATABASE_NAME}'
behavior).
---
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 67-72: The workflow currently uses identical credentials for
MASTER, MIGRATOR and APP users so tests won't validate permission boundaries;
update the environment variables (MASTER_NAME / MASTER_PASSWORD, MIGRATOR_NAME /
MIGRATOR_PASSWORD, APP_USER_NAME / APP_USER_PASSWORD) to use distinct usernames
and passwords (and mirror the same distinct values in the second block
referenced around lines 125-130) so CI exercises the different permission sets
and reveals regressions in the setup/alias wiring.
In `@Dockerfile`:
- Line 18: The Dockerfile installs mariadb-client twice: once in the common
stage and again in the run-common stage; remove the redundant installation from
the run-common stage so mariadb-client is only installed in the common stage.
Locate the two stages named "common" and "run-common" in the Dockerfile, and
delete the mariadb-client package entry from the apt-get install list in the
run-common stage (leaving the installation in the common stage intact); verify
no other dependent lines in run-common assume a separate install step.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 592e1ec7-0909-4d16-bfcb-2545929d08dc
📒 Files selected for processing (10)
.env.github/workflows/test.ymlDockerfileMakefiledocker-entrypoint.shsp7_db_setup_check.shspecifyweb/backend/permissions/initialize.pyspecifyweb/settings/__init__.pyspecifyweb/settings/specify_settings.pyspecifyweb/specify/management/commands/base_specify_migration.py
Comment on lines
+91
to
+95
| USER_EXISTS=$(mysql -h "$DB_HOST" -P "$DB_PORT" -u "$MASTER_USER_NAME" --password="$MASTER_USER_PASSWORD" -sse "SELECT COUNT(*) FROM mysql.user WHERE user = '$MIGRATOR_NAME' AND host = '$MIGRATOR_USER_HOST';") | ||
| if [[ "$USER_EXISTS" -eq 0 && "$APP_USER_NAME" != "root" ]]; then | ||
| echo "Creating user '$MIGRATOR_NAME'..." | ||
| echo "Executing: mysql -h \"$DB_HOST\" -P \"$DB_PORT\" -u \"$MASTER_USER_NAME\" --password=\"<hidden>\" -e \"CREATE USER '${MIGRATOR_NAME}'@'${MIGRATOR_USER_HOST}' IDENTIFIED BY '<hidden>';\"" | ||
| if mysql -h "$DB_HOST" -P "$DB_PORT" -u "$MASTER_USER_NAME" --password="$MASTER_USER_PASSWORD" -e "CREATE USER '${MIGRATOR_NAME}'@'${MIGRATOR_USER_HOST}' IDENTIFIED BY '${MIGRATOR_PASSWORD}';"; then |
There was a problem hiding this comment.
Check the migrator account, not the app account, before CREATE USER.
The guard is using APP_USER_NAME, but this branch creates MIGRATOR_NAME. If MIGRATOR_NAME=root, Line 95 still tries to create root, which breaks the root fallback case.
Suggested fix
-if [[ "$USER_EXISTS" -eq 0 && "$APP_USER_NAME" != "root" ]]; then
+if [[ "$USER_EXISTS" -eq 0 && "$MIGRATOR_NAME" != "root" ]]; then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| USER_EXISTS=$(mysql -h "$DB_HOST" -P "$DB_PORT" -u "$MASTER_USER_NAME" --password="$MASTER_USER_PASSWORD" -sse "SELECT COUNT(*) FROM mysql.user WHERE user = '$MIGRATOR_NAME' AND host = '$MIGRATOR_USER_HOST';") | |
| if [[ "$USER_EXISTS" -eq 0 && "$APP_USER_NAME" != "root" ]]; then | |
| echo "Creating user '$MIGRATOR_NAME'..." | |
| echo "Executing: mysql -h \"$DB_HOST\" -P \"$DB_PORT\" -u \"$MASTER_USER_NAME\" --password=\"<hidden>\" -e \"CREATE USER '${MIGRATOR_NAME}'@'${MIGRATOR_USER_HOST}' IDENTIFIED BY '<hidden>';\"" | |
| if mysql -h "$DB_HOST" -P "$DB_PORT" -u "$MASTER_USER_NAME" --password="$MASTER_USER_PASSWORD" -e "CREATE USER '${MIGRATOR_NAME}'@'${MIGRATOR_USER_HOST}' IDENTIFIED BY '${MIGRATOR_PASSWORD}';"; then | |
| USER_EXISTS=$(mysql -h "$DB_HOST" -P "$DB_PORT" -u "$MASTER_USER_NAME" --password="$MASTER_USER_PASSWORD" -sse "SELECT COUNT(*) FROM mysql.user WHERE user = '$MIGRATOR_NAME' AND host = '$MIGRATOR_USER_HOST';") | |
| if [[ "$USER_EXISTS" -eq 0 && "$MIGRATOR_NAME" != "root" ]]; then | |
| echo "Creating user '$MIGRATOR_NAME'..." | |
| echo "Executing: mysql -h \"$DB_HOST\" -P \"$DB_PORT\" -u \"$MASTER_USER_NAME\" --password=\"<hidden>\" -e \"CREATE USER '${MIGRATOR_NAME}'@'${MIGRATOR_USER_HOST}' IDENTIFIED BY '<hidden>';\"" | |
| if mysql -h "$DB_HOST" -P "$DB_PORT" -u "$MASTER_USER_NAME" --password="$MASTER_USER_PASSWORD" -e "CREATE USER '${MIGRATOR_NAME}'@'${MIGRATOR_USER_HOST}' IDENTIFIED BY '${MIGRATOR_PASSWORD}';"; then |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sp7_db_setup_check.sh` around lines 91 - 95, The condition guarding the
CREATE USER branch is checking APP_USER_NAME but the branch creates
MIGRATOR_NAME; change the guard to check MIGRATOR_NAME instead (i.e., use
MIGRATOR_NAME != "root") so we verify the migrator account before attempting
CREATE USER; keep the existing USER_EXISTS check and CREATE USER call
(MIGRATOR_NAME, MIGRATOR_USER_HOST, MIGRATOR_PASSWORD) unchanged so the root
fallback still works.
Comment on lines
+195
to
+239
| # Helper: check if a single GRANT line contains all REQUIRED_PRIVS | ||
| has_all_privs_in_line() { | ||
| local line="$1" | ||
| local missing=() | ||
| for p in "${REQUIRED_PRIVS[@]}"; do | ||
| # match whole words; spaces already canonicalized | ||
| if ! grep -qiE "(^|[, ])${p}(,| |$)" <<<"$line"; then | ||
| missing+=("$p") | ||
| fi | ||
| done | ||
| if [[ ${#missing[@]} -eq 0 ]]; then | ||
| return 0 | ||
| else | ||
| return 1 | ||
| fi | ||
| } | ||
|
|
||
| # Evaluate each grant line | ||
| for g in "${APP_GRANTS_LINES[@]}"; do | ||
| # If global ALL PRIVILEGES, good enough | ||
| if grep -qiE "^GRANT .*ALL PRIVILEGES.* ON \*\.\* TO " <<<"$g"; then | ||
| app_has_required_permissions=true; break | ||
| fi | ||
|
|
||
| # If global with at least required subset | ||
| if grep -qiE " ON \*\.\* TO " <<<"$g" && has_all_privs_in_line "$g"; then | ||
| app_has_required_permissions=true; break | ||
| fi | ||
|
|
||
| # If DB-scoped to this database and has required subset | ||
| if grep -qiE " ON (\`?${DB_NAME}\`?)\.\* TO " <<<"$g" && has_all_privs_in_line "$g"; then | ||
| app_has_required_permissions=true; break | ||
| fi | ||
| done | ||
|
|
||
| if [[ "$app_has_required_permissions" == true ]]; then | ||
| echo "Verified: '${APP_USER_NAME}'@'${APP_USER_HOST}' has required privileges on '${DB_NAME}'." | ||
| else | ||
| echo "Error: '${APP_USER_NAME}'@'${APP_USER_HOST}' lacks required privileges on '${DB_NAME}'." | ||
| echo "Required (any one GRANT must include all of): ${REQUIRED_PRIVS[*]}" | ||
| echo "Grants found:" | ||
| echo "$APP_GRANTS_RAW" | ||
| APP_USER_NAME="$MIGRATOR_NAME" | ||
| APP_USER_PASSWORD="$MIGRATOR_PASSWORD" | ||
| fi |
There was a problem hiding this comment.
Union matching app grants before deciding to fall back.
The verifier currently requires one GRANT line to contain the entire REQUIRED_PRIVS set. Existing installs can express a valid permission set across multiple grant statements, so this can falsely fail and promote the app to migrator credentials on Lines 237-238, weakening the least-privilege boundary this PR is adding.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sp7_db_setup_check.sh` around lines 195 - 239, The verifier currently expects
a single GRANT line to include all REQUIRED_PRIVS; instead, build a union of
privileges from all relevant grant lines before deciding. Modify the loop over
APP_GRANTS_LINES to, for each line that matches a relevant scope (global ON *.*
TO or DB-scoped ON `?${DB_NAME}`?.* TO), extract the privilege list portion
(text between "GRANT" and "ON"), normalize commas/spaces, and add each privilege
into a cumulative set (associative array or deduped list); after processing all
lines, check that every REQUIRED_PRIVS member is present in that union and set
app_has_required_permissions accordingly (rather than calling
has_all_privs_in_line per-line). Keep APP_GRANTS_RAW, REQUIRED_PRIVS,
APP_GRANTS_LINES, DB_NAME, app_has_required_permissions and the final fallback
logic intact.
Comment on lines
+33
to
+34
| except: | ||
| alias = 'master' |
There was a problem hiding this comment.
Avoid bare except clauses.
Bare except: catches all exceptions including KeyboardInterrupt and SystemExit, which can mask real errors and make debugging difficult. Use specific exception types.
🛠️ Suggested fix
- except:
+ except Exception:
alias = 'master'- except:
+ except Exception:
exists = FalseAlso applies to: 46-47
🧰 Tools
🪛 Ruff (0.15.14)
[error] 33-33: Do not use bare except
(E722)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@specifyweb/specify/management/commands/base_specify_migration.py` around
lines 33 - 34, The bare except that sets alias = 'master' should be replaced
with a narrow exception handler: catch the specific exceptions that can occur
when computing alias (e.g., IndexError, KeyError, ValueError) and optionally log
the exception before falling back to 'master' (e.g., except (IndexError,
KeyError, ValueError) as e: logger.warning("... %s", e); alias = 'master'). Do
the same replacement for the second occurrence at lines 46-47 so both bare
except blocks are removed in favor of specific exception handling and logging.
github-project-automation
Bot
moved this from 📋Back Log
to Dev Attention Needed
in General Tester Board
…L values before interpolation
Contributor
Author
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (2)
sp7_db_setup_check.sh (2)
266-271:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUnion app privileges across all
SHOW GRANTSlines.MariaDB can split a valid permission set across multiple
GRANTstatements. The current per-line check can reject a correct app account on existing installs and spuriously enter the fallback path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sp7_db_setup_check.sh` around lines 266 - 271, The current loop checks each grant line independently using grant_line_has_required_privs and may fail when privileges are split across multiple SHOW GRANTS lines; instead accumulate/union privileges from all APP_GRANTS_LINES and then test the combined set against APP_REQUIRED_PRIVS. Update the logic that iterates APP_GRANTS_LINES to extract privileges from each line (reusing or adding a helper used by grant_line_has_required_privs), merge them into a single collection, and set app_has_required_permissions=true only if the union contains all APP_REQUIRED_PRIVS; keep references to APP_GRANTS_LINES, grant_line_has_required_privs (or the new extractor), APP_REQUIRED_PRIVS, and app_has_required_permissions so reviewers can locate the change.
167-181:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCheck
MIGRATOR_NAMEin this guard.This branch creates
${MIGRATOR_NAME}, but the root special-case is checking${APP_USER_NAME}. If the migrator falls back toroot, the script still tries to createroot@'%'.🔧 One-line fix
-if [[ "$USER_EXISTS" -eq 0 && "$APP_USER_NAME" != "root" ]]; then +if [[ "$USER_EXISTS" -eq 0 && "$MIGRATOR_NAME" != "root" ]]; then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sp7_db_setup_check.sh` around lines 167 - 181, The guard is checking "$APP_USER_NAME" but the block creates the migrator user (${MIGRATOR_NAME}/${SQL_MIGRATOR_NAME}), so if the migrator resolves to root the script will still attempt to create root; change the conditional to check the migrator variable instead (e.g. replace "$APP_USER_NAME" != "root" with "$MIGRATOR_NAME" != "root" or "$SQL_MIGRATOR_NAME" != "root" to match the variables used in USER_EXISTS and CREATE USER), ensuring the check uses the same migrator identifier as in USER_EXISTS/CREATE USER.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Around line 67-72: The CI is provisioning only the default MYSQL_* user, so
fix the env var values so the "master" and migrator aliases map to the actual
privileged account: set MASTER_NAME (and MIGRATOR_NAME if you want migrator
privileged) to "root" and set MASTER_PASSWORD (and MIGRATOR_PASSWORD) to the
same value as MYSQL_ROOT_PASSWORD, while keeping APP_USER_NAME/APP_USER_PASSWORD
pointing to the unprivileged MYSQL_USER/MYSQL_PASSWORD; update the workflow env
block that defines
MASTER_NAME/MASTER_PASSWORD/MIGRATOR_NAME/MIGRATOR_PASSWORD/APP_USER_NAME/APP_USER_PASSWORD
and regenerate the specifyweb/settings/local_specify_settings.py so the
master/migrator aliases refer to the real root credentials instead of the
non‑privileged MYSQL_USER.
In `@docker-entrypoint.sh`:
- Around line 11-17: The entrypoint currently wraps ./sp7_db_setup_check.sh with
set +e but never checks its exit status, allowing the container to continue on
failures; update docker-entrypoint.sh to fail fast by either removing set +e
around ./sp7_db_setup_check.sh or, if you need to preserve it, capture the
script's exit code (e.g., rc=$?) immediately after running
./sp7_db_setup_check.sh and if rc != 0 call echo with an error and exit $rc so
the container stops when sp7_db_setup_check.sh fails; reference the invocation
of ./sp7_db_setup_check.sh in docker-entrypoint.sh and ensure set -e is active
after this check.
In `@Makefile`:
- Around line 17-19: The django_migrations Makefile target diverges from startup
flow because it never uses the --use-override flag; update the target
(django_migrations) so both manage.py calls include a conditional override flag
(e.g., add a MAKE variable like USE_OVERRIDE that defaults empty and append
$(USE_OVERRIDE) to the base_specify_migration and migrate invocations), so
callers can run make django_migrations USE_OVERRIDE=--use-override to match
sp7_db_setup_check.sh behavior.
In `@sp7_db_setup_check.sh`:
- Around line 117-118: The app runtime privilege list (APP_REQUIRED_PRIVS) still
contains schema-changing rights; remove "ALTER" and "INDEX" (and any other
schema mutation privileges) from APP_REQUIRED_PRIVS and ensure they remain in
MIGRATION_REQUIRED_PRIVS so only the migrator/privileged path has schema-change
permissions; update both occurrences (the current declaration and the duplicate
around lines 243-246) to reflect this split and keep MIGRATION_REQUIRED_PRIVS as
the authoritative list for ALTER/INDEX.
- Around line 13-19: The assignments to MIGRATOR_NAME, MIGRATOR_PASSWORD,
APP_USER_NAME and APP_USER_PASSWORD fail under set -u when the env vars are
absent; change those lines to use safe parameter expansion (e.g.
${MIGRATOR_NAME:-} etc.) so they evaluate to an empty string instead of causing
an “unbound variable” error and allow the later derivation logic to run; update
the occurrences in sp7_db_setup_check.sh for MIGRATOR_NAME, MIGRATOR_PASSWORD,
APP_USER_NAME and APP_USER_PASSWORD accordingly.
In `@specifyweb/backend/permissions/initialize.py`:
- Around line 87-96: The early-return table-existence check that protects
assign_users_to_roles() should be extracted into a shared helper (e.g., add a
function like sp6_tables_exist() or ensure_sp6_tables_present()) and then called
from both assign_users_to_roles() and assign_users_to_roles_during_testing();
move the current cursor-based SELECT COUNT(*) check (the information_schema
query that looks for 'specifyuser_spprincipal' and 'spuserrole') into that
helper so users_collections_for_sp6(...) is never invoked when those tables are
absent, and update initialize() to call the new helper in the test-only
assignment path as well.
In `@specifyweb/specify/management/commands/base_specify_migration.py`:
- Around line 38-53: The current probe around SELECT 1 FROM django_migrations
inside transaction.atomic(using=alias) swallows all exceptions via a bare except
and can hide non-“table missing” errors; change the logic in the block that uses
conn.cursor(), cursor.execute(...) and the exists flag to either use Django's DB
introspection (connection.introspection.table_names() or
connection.introspection.table_exists) to check for the django_migrations table,
or catch only the specific DB-API error that indicates “table does not exist”
(e.g. the MySQL error/exception type your backend raises) and re-raise any other
exceptions (or call transaction.set_rollback(True) before continuing) so you
don't mask permission/connection errors and so atomic(using=alias) behaves
correctly.
---
Duplicate comments:
In `@sp7_db_setup_check.sh`:
- Around line 266-271: The current loop checks each grant line independently
using grant_line_has_required_privs and may fail when privileges are split
across multiple SHOW GRANTS lines; instead accumulate/union privileges from all
APP_GRANTS_LINES and then test the combined set against APP_REQUIRED_PRIVS.
Update the logic that iterates APP_GRANTS_LINES to extract privileges from each
line (reusing or adding a helper used by grant_line_has_required_privs), merge
them into a single collection, and set app_has_required_permissions=true only if
the union contains all APP_REQUIRED_PRIVS; keep references to APP_GRANTS_LINES,
grant_line_has_required_privs (or the new extractor), APP_REQUIRED_PRIVS, and
app_has_required_permissions so reviewers can locate the change.
- Around line 167-181: The guard is checking "$APP_USER_NAME" but the block
creates the migrator user (${MIGRATOR_NAME}/${SQL_MIGRATOR_NAME}), so if the
migrator resolves to root the script will still attempt to create root; change
the conditional to check the migrator variable instead (e.g. replace
"$APP_USER_NAME" != "root" with "$MIGRATOR_NAME" != "root" or
"$SQL_MIGRATOR_NAME" != "root" to match the variables used in USER_EXISTS and
CREATE USER), ensuring the check uses the same migrator identifier as in
USER_EXISTS/CREATE USER.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a0548793-ef4f-4d7a-ac7c-e105ea21b274
📒 Files selected for processing (10)
.env.github/workflows/test.ymlDockerfileMakefiledocker-entrypoint.shsp7_db_setup_check.shspecifyweb/backend/permissions/initialize.pyspecifyweb/settings/__init__.pyspecifyweb/settings/specify_settings.pyspecifyweb/specify/management/commands/base_specify_migration.py
Comment on lines
+67
to
+72
| MASTER_NAME: MasterUser | ||
| MASTER_PASSWORD: MasterPassword | ||
| MIGRATOR_NAME: MasterUser | ||
| MIGRATOR_PASSWORD: MasterPassword | ||
| APP_USER_NAME: MasterUser | ||
| APP_USER_PASSWORD: MasterPassword |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Which environment variables does the official MariaDB Docker image honor for creating the initial root/user/database, and do custom variables like MASTER_NAME, MIGRATOR_NAME, or APP_USER_NAME provision users automatically?
💡 Result:
The official MariaDB Docker image uses specific environment variables to initialize the database and provision users upon the container's first startup [1][2]. These variables have no effect if the data directory already contains an initialized database [1][2]. Standard Environment Variables for Initialization To secure the root user, you must provide one of the following variables (or their FILE equivalents for secrets): MARIADB_ROOT_PASSWORD or MYSQL_ROOT_PASSWORD: Sets the password for the root superuser [1][2]. MARIADB_ROOT_PASSWORD_HASH or MYSQL_ROOT_PASSWORD_HASH: Sets the password hash for the root user [1][2]. MARIADB_RANDOM_ROOT_PASSWORD or MYSQL_RANDOM_ROOT_PASSWORD: Generates a random root password [1][2]. MARIADB_ALLOW_EMPTY_ROOT_PASSWORD or MYSQL_ALLOW_EMPTY_PASSWORD: Allows the root user to have an empty password [1][2]. Additional variables for user and database creation: MARIADB_DATABASE or MYSQL_DATABASE: Creates the specified database on startup [1][2][3]. MARIADB_USER or MYSQL_USER: Creates an additional user [3][4]. MARIADB_PASSWORD or MYSQL_PASSWORD: Sets the password for the created user [3][4]. Note: For versions 10.6 and above, the official image prefers MARIADB variables, though it maintains backward compatibility with MYSQL_* equivalents [1][5]. Using *_FILE suffixes (e.g., MARIADB_ROOT_PASSWORD_FILE) is the recommended security practice for handling sensitive credentials via mounted files [1][2]. Custom Variables Custom environment variables such as MASTER_NAME, MIGRATOR_NAME, or APP_USER_NAME do not automatically provision users or databases [1][2]. The initialization script only recognizes the predefined set of standard variables listed above [5]. To provision users or databases with custom names or configurations not supported by these standard environment variables, you should utilize initialization scripts [2][3][6]. By placing.sql,.sh,.sql.gz, or.sql.xz files into the /docker-entrypoint-initdb.d/ directory within the container, you can execute custom SQL commands to create users, grant privileges, or initialize schemas during the initial startup process [2][3][6].
Citations:
- 1: https://mariadb.com/docs/server/server-management/automated-mariadb-deployment-and-administration/docker-and-mariadb/mariadb-server-docker-official-image-environment-variables
- 2: https://hub.docker.com/_/mariadb
- 3: https://dev.to/piteradyson/mariadb-docker-setup-running-mariadb-in-docker-containers-complete-guide-17ba
- 4: https://dev.to/exploringmylifeworks/install-joget-with-docker-and-mysql-5242
- 5: https://github.com/MariaDB/mariadb-docker/blob/56ef6d9f842e1ddf50e4359625a6e5cef1748c38/docker-entrypoint.sh
- 6: https://dev.to/davidtio/customizing-docker-images-write-your-first-dockerfile-2026-5ga7
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show relevant sections of the workflow around the referenced lines
echo "=== .github/workflows/test.yml (context around lines 50-110) ==="
nl -ba .github/workflows/test.yml | sed -n '45,110p'
# Search for settings generation and the exact echoed vars around local_specify_settings.py
echo "=== Search for local_specify_settings.py in workflow ==="
rg -n "local_specify_settings\.py" .github/workflows/test.yml
echo "=== Context around settings generation (lines 110-170) ==="
nl -ba .github/workflows/test.yml | sed -n '110,190p'
# If the workflow writes a python settings file, show exact commands near it
echo "=== Show any echo/tee lines setting MASTER_/MIGRATOR_/APP_USER_ ==="
rg -n "MASTER_NAME|MASTER_PASSWORD|MIGRATOR_NAME|APP_USER_NAME|MIGRATOR_PASSWORD|APP_USER_PASSWORD" .github/workflows/test.ymlRepository: specify/specify7
Length of output: 186
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show relevant sections of the workflow around the referenced lines
echo "=== .github/workflows/test.yml (context around lines 50-110) ==="
nl -ba .github/workflows/test.yml | sed -n '45,110p'
# Search for settings generation and the exact echoed vars around local_specify_settings.py
echo "=== Search for local_specify_settings.py in workflow ==="
rg -n "local_specify_settings\.py" .github/workflows/test.yml || true
echo "=== Context around settings generation (lines 110-210) ==="
nl -ba .github/workflows/test.yml | sed -n '110,210p'
echo "=== Any echo/assignment lines for MASTER_/MIGRATOR_/APP_USER_ in workflow ==="
rg -n "MASTER_NAME|MASTER_PASSWORD|MIGRATOR_NAME|MIGRATOR_PASSWORD|APP_USER_NAME|APP_USER_PASSWORD" .github/workflows/test.yml || trueRepository: specify/specify7
Length of output: 186
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Workflow snippet: .github/workflows/test.yml around lines 55-90 ==="
sed -n '55,90p' .github/workflows/test.yml | awk '{printf "%6d %s\n", NR+54, $0}'
echo "=== Workflow snippet: .github/workflows/test.yml around lines 110-160 ==="
sed -n '110,160p' .github/workflows/test.yml | awk '{printf "%6d %s\n", NR+109, $0}'
echo "=== Search for local_specify_settings.py generation in workflow ==="
rg -n "local_specify_settings\.py" .github/workflows/test.yml || true
echo "=== Search for MASTER_/MIGRATOR_/APP_USER_ in workflow ==="
rg -n "MASTER_NAME|MASTER_PASSWORD|MIGRATOR_NAME|MIGRATOR_PASSWORD|APP_USER_NAME|APP_USER_PASSWORD" .github/workflows/test.yml || true
echo "=== Search for MariaDB env / MYSQL_* in workflow ==="
rg -n "MARIADB_|MYSQL_" .github/workflows/test.yml || trueRepository: specify/specify7
Length of output: 6518
CI: master alias isn’t backed by a distinct privileged MariaDB user
- The MariaDB container initialization uses only
MYSQL_*env vars (MYSQL_USER/MYSQL_PASSWORD/MYSQL_DATABASEandMYSQL_ROOT_PASSWORD); customMASTER_NAME/MIGRATOR_NAME/APP_USER_NAMEdon’t provision users automatically. - The workflow then generates
specifyweb/settings/local_specify_settings.pywithMASTER_NAME,MIGRATOR_NAME, andAPP_USER_NAMEall set toMasterUser(the same account created viaMYSQL_USER), so the “master” path isn’t privileged in CI.
🔧 Minimal CI-safe adjustment
- MASTER_NAME: MasterUser
- MASTER_PASSWORD: MasterPassword
+ MASTER_NAME: root
+ MASTER_PASSWORD: password
...
- echo "MASTER_NAME = 'MasterUser'" >> specifyweb/settings/local_specify_settings.py
- echo "MASTER_PASSWORD = 'MasterPassword'" >> specifyweb/settings/local_specify_settings.py
+ echo "MASTER_NAME = 'root'" >> specifyweb/settings/local_specify_settings.py
+ echo "MASTER_PASSWORD = 'password'" >> specifyweb/settings/local_specify_settings.py🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/test.yml around lines 67 - 72, The CI is provisioning only
the default MYSQL_* user, so fix the env var values so the "master" and migrator
aliases map to the actual privileged account: set MASTER_NAME (and MIGRATOR_NAME
if you want migrator privileged) to "root" and set MASTER_PASSWORD (and
MIGRATOR_PASSWORD) to the same value as MYSQL_ROOT_PASSWORD, while keeping
APP_USER_NAME/APP_USER_PASSWORD pointing to the unprivileged
MYSQL_USER/MYSQL_PASSWORD; update the workflow env block that defines
MASTER_NAME/MASTER_PASSWORD/MIGRATOR_NAME/MIGRATOR_PASSWORD/APP_USER_NAME/APP_USER_PASSWORD
and regenerate the specifyweb/settings/local_specify_settings.py so the
master/migrator aliases refer to the real root credentials instead of the
non‑privileged MYSQL_USER.
Comment on lines
11
to
17
| echo "Applying Django migrations." | ||
| set +e | ||
| ve/bin/python manage.py base_specify_migration | ||
| ve/bin/python manage.py migrate | ||
| ./sp7_db_setup_check.sh # Setup db users and run migrations | ||
| # ve/bin/python manage.py base_specify_migration | ||
| # ve/bin/python manage.py migrate | ||
| # ve/bin/python manage.py run_key_migration_functions # Uncomment if you want the key migration functions to run on startup. | ||
| set -e |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
nl -ba docker-entrypoint.sh | sed -n '7,18p'
# Expect: `set +e` before `./sp7_db_setup_check.sh` and no exit-code check afterwards.Repository: specify/specify7
Length of output: 101
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print the relevant portion with line numbers (without using `nl`)
awk 'NR>=1 && NR<=120 {printf "%5d %s\n", NR, $0}' docker-entrypoint.sh | sed -n '1,120p' 1>/dev/null
awk 'NR>=7 && NR<=25 {printf "%5d %s\n", NR, $0}' docker-entrypoint.sh
# Focused greps
rg -n --fixed-string "set +e" docker-entrypoint.sh || true
rg -n --fixed-string "set -e" docker-entrypoint.sh || true
rg -n --fixed-string "./sp7_db_setup_check.sh" docker-entrypoint.sh || true
rg -n --regexp "\\$\\?" docker-entrypoint.sh || true
rg -n --regexp "sp7_db_setup_check\\.sh.*\\|\\|" docker-entrypoint.sh || true
rg -n --regexp "if .*sp7_db_setup_check\\.sh" docker-entrypoint.sh || trueRepository: specify/specify7
Length of output: 2075
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== docker-entrypoint.sh =="
awk '{printf "%5d %s\n", NR, $0}' docker-entrypoint.sh
echo
echo "== searches (use correct rg flags) =="
rg -n -F "set +e" docker-entrypoint.sh || true
rg -n -F "./sp7_db_setup_check.sh" docker-entrypoint.sh || true
rg -n -F "set -e" docker-entrypoint.sh || true
echo
echo "== sp7_db_setup_check.sh (show beginning + any explicit exits/errors) =="
wc -l sp7_db_setup_check.sh
awk 'NR<=200 {printf "%5d %s\n", NR, $0}' sp7_db_setup_check.sh
echo
rg -n "exit[[:space:]]|set[[:space:]]+-e|set[[:space:]]+-o|trap|FAIL|ERROR|migrate" sp7_db_setup_check.sh || trueRepository: specify/specify7
Length of output: 10089
Fail entrypoint fast when ./sp7_db_setup_check.sh fails (DB/user/migrations)
docker-entrypoint.sh disables fail-fast with set +e around ./sp7_db_setup_check.sh and then re-enables it, but it never checks the script’s exit code. Since sp7_db_setup_check.sh uses set -euo pipefail and explicitly exit 1 on failures (e.g., missing env, DB/user creation, and migration steps), the container can still continue to exec "$@" (Gunicorn) after a failed DB/migration.
✅ Let the setup script fail the container
- set +e
./sp7_db_setup_check.sh # Setup db users and run migrations
# ve/bin/python manage.py base_specify_migration
# ve/bin/python manage.py migrate
# ve/bin/python manage.py run_key_migration_functions # Uncomment if you want the key migration functions to run on startup.
- set -e📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| echo "Applying Django migrations." | |
| set +e | |
| ve/bin/python manage.py base_specify_migration | |
| ve/bin/python manage.py migrate | |
| ./sp7_db_setup_check.sh # Setup db users and run migrations | |
| # ve/bin/python manage.py base_specify_migration | |
| # ve/bin/python manage.py migrate | |
| # ve/bin/python manage.py run_key_migration_functions # Uncomment if you want the key migration functions to run on startup. | |
| set -e | |
| echo "Applying Django migrations." | |
| ./sp7_db_setup_check.sh # Setup db users and run migrations | |
| # ve/bin/python manage.py base_specify_migration | |
| # ve/bin/python manage.py migrate | |
| # ve/bin/python manage.py run_key_migration_functions # Uncomment if you want the key migration functions to run on startup. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-entrypoint.sh` around lines 11 - 17, The entrypoint currently wraps
./sp7_db_setup_check.sh with set +e but never checks its exit status, allowing
the container to continue on failures; update docker-entrypoint.sh to fail fast
by either removing set +e around ./sp7_db_setup_check.sh or, if you need to
preserve it, capture the script's exit code (e.g., rc=$?) immediately after
running ./sp7_db_setup_check.sh and if rc != 0 call echo with an error and exit
$rc so the container stops when sp7_db_setup_check.sh fails; reference the
invocation of ./sp7_db_setup_check.sh in docker-entrypoint.sh and ensure set -e
is active after this check.
Comment on lines
17
to
+19
| django_migrations: | ||
| $(PYTHON) manage.py base_specify_migration | ||
| $(PYTHON) manage.py migrate | ||
| $(PYTHON) manage.py base_specify_migration --database=migrations | ||
| $(PYTHON) manage.py migrate --database=migrations |
There was a problem hiding this comment.
Keep this target aligned with the startup migration flow.
sp7_db_setup_check.sh uses --use-override for pre-existing databases, but make django_migrations always takes the non-override path. Running this target against an existing Specify database can now diverge from container startup or fail unexpectedly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Makefile` around lines 17 - 19, The django_migrations Makefile target
diverges from startup flow because it never uses the --use-override flag; update
the target (django_migrations) so both manage.py calls include a conditional
override flag (e.g., add a MAKE variable like USE_OVERRIDE that defaults empty
and append $(USE_OVERRIDE) to the base_specify_migration and migrate
invocations), so callers can run make django_migrations
USE_OVERRIDE=--use-override to match sp7_db_setup_check.sh behavior.
Comment on lines
+13
to
+19
| MIGRATOR_NAME="${MIGRATOR_NAME}" | ||
| MIGRATOR_PASSWORD="${MIGRATOR_PASSWORD}" | ||
| MIGRATOR_USER_HOST="%" | ||
| DB_ROOT_PASSWORD="${MYSQL_ROOT_PASSWORD}" | ||
| DB_NAME="${DATABASE_NAME}" | ||
| APP_USER_NAME="${APP_USER_NAME}" | ||
| APP_USER_PASSWORD="${APP_USER_PASSWORD}" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
bash -uc 'unset MIGRATOR_NAME APP_USER_NAME; set -u; MIGRATOR_NAME="${MIGRATOR_NAME}"'
# Expect: non-zero exit, which is what the current script would do before reaching its fallback block.Repository: specify/specify7
Length of output: 108
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la
echo "----"
fd -a -t f "sp7_db_setup_check.sh" . || true
echo "----"
FILE="$(fd -a -t f "sp7_db_setup_check.sh" . | head -n 1 || true)"
if [ -z "${FILE}" ]; then
echo "sp7_db_setup_check.sh not found"
exit 1
fi
echo "Using: $FILE"
echo "----"
nl -ba "$FILE" | sed -n '1,120p'Repository: specify/specify7
Length of output: 2376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="sp7_db_setup_check.sh"
if [ ! -f "$FILE" ]; then
FILE="$(fd -a -t f "^sp7_db_setup_check\.sh$" . | head -n1)"
fi
echo "Using: $FILE"
# Print lines 1-80 with line numbers using awk (nl may not exist)
awk 'NR>=1 && NR<=90 { printf "%4d %s\n", NR, $0 }' "$FILE"
echo "----"
# Specifically show referenced region
awk 'NR>=1 && NR<=70 { printf "%4d %s\n", NR, $0 }' "$FILE" | sed -n '10,60p'Repository: specify/specify7
Length of output: 4942
Fix set -u unbound-variable failure for migrator/app user role env vars (lines 13-19)
With set -u, ${MIGRATOR_NAME}, ${MIGRATOR_PASSWORD}, ${APP_USER_NAME}, and ${APP_USER_PASSWORD} will terminate the script with “unbound variable” when those env vars are absent, so the fallback/derivation logic (lines 22-41) never runs.
🩹 Minimal fix
-MIGRATOR_NAME="${MIGRATOR_NAME}"
-MIGRATOR_PASSWORD="${MIGRATOR_PASSWORD}"
+MIGRATOR_NAME="${MIGRATOR_NAME:-}"
+MIGRATOR_PASSWORD="${MIGRATOR_PASSWORD:-}"
...
-APP_USER_NAME="${APP_USER_NAME}"
-APP_USER_PASSWORD="${APP_USER_PASSWORD}"
+APP_USER_NAME="${APP_USER_NAME:-}"
+APP_USER_PASSWORD="${APP_USER_PASSWORD:-}"🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 13-13: This variable is assigned to itself, so the assignment does nothing.
(SC2269)
[info] 14-14: This variable is assigned to itself, so the assignment does nothing.
(SC2269)
[info] 18-18: This variable is assigned to itself, so the assignment does nothing.
(SC2269)
[info] 19-19: This variable is assigned to itself, so the assignment does nothing.
(SC2269)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sp7_db_setup_check.sh` around lines 13 - 19, The assignments to
MIGRATOR_NAME, MIGRATOR_PASSWORD, APP_USER_NAME and APP_USER_PASSWORD fail under
set -u when the env vars are absent; change those lines to use safe parameter
expansion (e.g. ${MIGRATOR_NAME:-} etc.) so they evaluate to an empty string
instead of causing an “unbound variable” error and allow the later derivation
logic to run; update the occurrences in sp7_db_setup_check.sh for MIGRATOR_NAME,
MIGRATOR_PASSWORD, APP_USER_NAME and APP_USER_PASSWORD accordingly.
Comment on lines
+117
to
+118
| MIGRATION_REQUIRED_PRIVS=("SELECT" "INSERT" "UPDATE" "DELETE" "CREATE" "ALTER" "INDEX" "DROP") | ||
| APP_REQUIRED_PRIVS=("SELECT" "INSERT" "UPDATE" "ALTER" "INDEX" "DELETE" "CREATE TEMPORARY TABLES" "LOCK TABLES" "EXECUTE") |
There was a problem hiding this comment.
The runtime app grant still includes schema-changing privileges.
ALTER and INDEX let the application account mutate schema, which collapses the least-privilege boundary this PR is adding. Those permissions should stay on the migrator/master path.
🔐 Tighten the app privilege set
-APP_REQUIRED_PRIVS=("SELECT" "INSERT" "UPDATE" "ALTER" "INDEX" "DELETE" "CREATE TEMPORARY TABLES" "LOCK TABLES" "EXECUTE")
+APP_REQUIRED_PRIVS=("SELECT" "INSERT" "UPDATE" "DELETE" "CREATE TEMPORARY TABLES" "LOCK TABLES" "EXECUTE")
...
- if ! mysql -h "$DB_HOST" -P "$DB_PORT" -u "$MASTER_USER_NAME" --password="$MASTER_USER_PASSWORD" -e "GRANT SELECT, INSERT, UPDATE, ALTER, INDEX, DELETE, CREATE TEMPORARY TABLES, LOCK TABLES, EXECUTE ON ${SQL_DB_IDENTIFIER}.* TO ${SQL_APP_USER_NAME}@${SQL_APP_USER_HOST}; FLUSH PRIVILEGES;"; then
+ if ! mysql -h "$DB_HOST" -P "$DB_PORT" -u "$MASTER_USER_NAME" --password="$MASTER_USER_PASSWORD" -e "GRANT SELECT, INSERT, UPDATE, DELETE, CREATE TEMPORARY TABLES, LOCK TABLES, EXECUTE ON ${SQL_DB_IDENTIFIER}.* TO ${SQL_APP_USER_NAME}@${SQL_APP_USER_HOST}; FLUSH PRIVILEGES;"; thenAlso applies to: 243-246
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sp7_db_setup_check.sh` around lines 117 - 118, The app runtime privilege list
(APP_REQUIRED_PRIVS) still contains schema-changing rights; remove "ALTER" and
"INDEX" (and any other schema mutation privileges) from APP_REQUIRED_PRIVS and
ensure they remain in MIGRATION_REQUIRED_PRIVS so only the migrator/privileged
path has schema-change permissions; update both occurrences (the current
declaration and the duplicate around lines 243-246) to reflect this split and
keep MIGRATION_REQUIRED_PRIVS as the authoritative list for ALTER/INDEX.
Comment on lines
+87
to
+96
| results = [] | ||
| with connection.cursor() as cursor: | ||
| cursor.execute(""" | ||
| SELECT COUNT(*) | ||
| FROM information_schema.tables | ||
| WHERE table_name IN ('specifyuser_spprincipal', 'spuserrole') | ||
| AND table_schema = DATABASE(); | ||
| """) | ||
| if cursor.fetchone()[0] < 2: | ||
| return # Newly created sp7 databases don't have these sp6 specific tables. |
There was a problem hiding this comment.
Mirror this legacy-table guard in the test-only assignment path.
This early return only protects assign_users_to_roles(). During test DB setup, initialize() switches to assign_users_to_roles_during_testing() on Lines 41-44, and that helper still calls users_collections_for_sp6(...) without any table-existence check. Fresh SP7 test databases can therefore still fail during permissions initialization even though this branch now returns safely.
Please extract this preflight into a shared helper and use it from both assignment functions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@specifyweb/backend/permissions/initialize.py` around lines 87 - 96, The
early-return table-existence check that protects assign_users_to_roles() should
be extracted into a shared helper (e.g., add a function like sp6_tables_exist()
or ensure_sp6_tables_present()) and then called from both
assign_users_to_roles() and assign_users_to_roles_during_testing(); move the
current cursor-based SELECT COUNT(*) check (the information_schema query that
looks for 'specifyuser_spprincipal' and 'spuserrole') into that helper so
users_collections_for_sp6(...) is never invoked when those tables are absent,
and update initialize() to call the new helper in the test-only assignment path
as well.
Comment on lines
+38
to
+53
| with transaction.atomic(using=alias): | ||
| with conn.cursor() as cursor: | ||
| # Check django table | ||
| try: | ||
| cursor.execute(""" | ||
| SELECT 1 | ||
| FROM django_migrations | ||
| LIMIT 1; | ||
| """) | ||
| exists = True | ||
| except: | ||
| exists = False | ||
|
|
||
| if not exists: | ||
| # Check if the django_migrations table exists and create it if it doesn't | ||
| cursor.execute(""" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and print the relevant section with line numbers
FILE="specifyweb/specify/management/commands/base_specify_migration.py"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE"
exit 1
fi
nl -ba "$FILE" | sed -n '1,140p'
# Extract only the snippet area for quick reading
nl -ba "$FILE" | sed -n '30,110p'
# Search for other uses of django_migrations probe / CREATE TABLE in the repo
rg -n "django_migrations" specifyweb/specify/management/commands/base_specify_migration.py
rg -n "transaction\.atomic\(using=.*\)" specifyweb/specify/management/commands/base_specify_migration.py
rg -n "CREATE TABLE IF NOT EXISTS `django_migrations`|CREATE TABLE IF NOT EXISTS \"django_migrations\"|CREATE TABLE.*django_migrations" -S specifyweb | head -n 50
# Check Django version constraints (if available)
rg -n "Django==|Django>=" -S . || true
ls -laRepository: specify/specify7
Length of output: 104
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="specifyweb/specify/management/commands/base_specify_migration.py"
nl -ba "$FILE" | sed -n '1,120p'
nl -ba "$FILE" | sed -n '120,220p' || true
rg -n "django_migrations" specifyweb/specify/management/commands/base_specify_migration.py
rg -n "transaction\.atomic" specifyweb/specify/management/commands/base_specify_migration.py
# Look for requirements / pyproject to infer Django version
ls
fd -t f "pyproject.toml" -a . 2>/dev/null | head -n 20 || true
fd -t f "requirements*.txt" -a . 2>/dev/null | head -n 20 || true
fd -t f "Pipfile*" -a . 2>/dev/null | head -n 20 || true
rg -n "Django" pyproject.toml requirements*.txt Pipfile* 2>/dev/null || trueRepository: specify/specify7
Length of output: 103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="specifyweb/specify/management/commands/base_specify_migration.py"
# Show relevant file sections
nl -ba "$FILE" | sed -n '1,120p'
nl -ba "$FILE" | sed -n '120,220p' || true
nl -ba "$FILE" | sed -n '220,320p' || true
# Targeted extracts
echo "---- django_migrations occurrences ----"
rg -n "django_migrations" "$FILE"
echo "---- transaction.atomic occurrences ----"
rg -n "transaction\.atomic" "$FILE"
# Repo-wide quick scan for the CREATE TABLE / existence probe
echo "---- CREATE TABLE occurrences ----"
rg -n "CREATE TABLE.*django_migrations" specifyweb | head -n 50
echo "---- Dependency guesses (Django version) ----"
ls
fd -t f "pyproject.toml" -a . 2>/dev/null | head -n 20 || true
fd -t f "requirements*.txt" -a . 2>/dev/null | head -n 20 || true
fd -t f "Pipfile*" -a . 2>/dev/null | head -n 20 || true
rg -n "Django" pyproject.toml requirements*.txt Pipfile* 2>/dev/null || trueRepository: specify/specify7
Length of output: 103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="specifyweb/specify/management/commands/base_specify_migration.py"
nl -ba "$FILE" | sed -n '1,160p'
rg -n "django_migrations" "$FILE"
rg -n "transaction\.atomic" "$FILE"
rg -n "CREATE TABLE.*django_migrations" specifyweb | head -n 50
# Check dependency files for Django version
for f in pyproject.toml requirements.txt requirements-dev.txt requirements*.txt Pipfile Pipfile.lock; do
if [ -f "$f" ]; then
echo "---- $f ----"
rg -n "Django" "$f" || true
fi
done
# Also list common filenames if not present
fd -t f "requirements*.txt" . | head -n 20
fd -t f "pyproject.toml" . | head -n 20Repository: specify/specify7
Length of output: 103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="specifyweb/specify/management/commands/base_specify_migration.py"
nl -ba "$FILE" | sed -n '1,200p'
echo "---- django_migrations occurrences ----"
rg -n "django_migrations" "$FILE"
echo "---- transaction.atomic occurrences ----"
rg -n "transaction\.atomic" "$FILE"
echo "---- CREATE TABLE occurrences ----"
rg -n "CREATE TABLE.*django_migrations" specifyweb | head -n 50
echo "---- Dependency files ----"
ls -la
fd -t f "pyproject.toml" . | head -n 20
fd -t f "requirements*.txt" . | head -n 20
fd -t f "Pipfile*" . | head -n 20
# Try to print the lines mentioning Django from common dependency files
for f in $(fd -t f "pyproject.toml" . 2>/dev/null; fd -t f "requirements*.txt" . 2>/dev/null; fd -t f "Pipfile*" . 2>/dev/null); do
echo "---- $f ----"
rg -n "Django" "$f" || true
doneRepository: specify/specify7
Length of output: 103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="specifyweb/specify/management/commands/base_specify_migration.py"
nl -ba "$FILE" | sed -n '1,220p'
echo "---- django_migrations occurrences ----"
rg -n "django_migrations" "$FILE"
echo "---- transaction.atomic occurrences ----"
rg -n "transaction\.atomic" "$FILE"
echo "---- CREATE TABLE occurrences ----"
rg -n "CREATE TABLE.*django_migrations" specifyweb | head -n 50
echo "---- Dependency hints (Django version) ----"
deps=$(fd -t f -g "requirements*.txt" . 2>/dev/null | head -n 20; true)
pp=$(fd -t f -g "pyproject.toml" . 2>/dev/null | head -n 20; true)
pipfiles=$(fd -t f -g "Pipfile*" . 2>/dev/null | head -n 20; true)
for f in $pp $deps $pipfiles; do
echo "---- $f ----"
rg -n "Django" "$f" || true
doneRepository: specify/specify7
Length of output: 103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="specifyweb/specify/management/commands/base_specify_migration.py"
nl -ba "$FILE" | sed -n '1,220p'
echo "---- django_migrations occurrences ----"
rg -n "django_migrations" "$FILE"
echo "---- transaction.atomic occurrences ----"
rg -n "transaction\.atomic" "$FILE"
echo "---- CREATE TABLE occurrences ----"
rg -n "CREATE TABLE.*django_migrations" specifyweb | head -n 50
echo "---- Dependency hints (Django version) ----"
for f in $(fd -t f -g "pyproject.toml" . 2>/dev/null | head -n 20); do
echo "---- $f ----"
rg -n "Django" "$f" || true
done
for f in $(fd -t f -g "requirements*.txt" . 2>/dev/null | head -n 20); do
echo "---- $f ----"
rg -n "Django" "$f" || true
done
for f in $(fd -t f -g "Pipfile*" . 2>/dev/null | head -n 20); do
echo "---- $f ----"
rg -n "Django" "$f" || true
doneRepository: specify/specify7
Length of output: 103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="specifyweb/specify/management/commands/base_specify_migration.py"
nl -ba "$FILE" | sed -n '1,220p'
echo "---- django_migrations occurrences ----"
rg -n "django_migrations" "$FILE"
echo "---- transaction.atomic occurrences ----"
rg -n "transaction\.atomic" "$FILE"
echo "---- CREATE TABLE occurrences ----"
rg -n "CREATE TABLE.*django_migrations" specifyweb | head -n 50
echo "---- Dependency hints (Django version) ----"
for f in $(fd -t f -g "pyproject.toml" . 2>/dev/null | head -n 20); do
echo "---- $f ----"
rg -n "Django" "$f" || true
done
for f in $(fd -t f -g "requirements*.txt" . 2>/dev/null | head -n 20); do
echo "---- $f ----"
rg -n "Django" "$f" || true
done
for f in $(fd -t f -g "Pipfile*" . 2>/dev/null | head -n 20); do
echo "---- $f ----"
rg -n "Django" "$f" || true
doneRepository: specify/specify7
Length of output: 103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="specifyweb/specify/management/commands/base_specify_migration.py"
# show only the section around the reported line numbers
nl -ba "$FILE" | sed -n '25,90p'
# show the CREATE TABLE block nearby
nl -ba "$FILE" | sed -n '90,170p'
echo "---- exact django_migrations-related lines ----"
rg -n "django_migrations" "$FILE" -n
echo "---- transaction.atomic-related lines ----"
rg -n "transaction\.atomic" "$FILE"
echo "---- dependency hints (Django version) ----"
# print only matched dependency lines
for f in $(fd -t f -g "pyproject.toml" . 2>/dev/null | head -n 5; fd -t f -g "requirements*.txt" . 2>/dev/null | head -n 10; fd -t f -g "Pipfile*" . 2>/dev/null | head -n 5); do
rg -n "Django" "$f" || true
doneRepository: specify/specify7
Length of output: 103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="specifyweb/specify/management/commands/base_specify_migration.py"
# Basic sanity checks
test -f "$FILE" && echo "Found: $FILE" || { echo "Missing: $FILE"; exit 1; }
echo "---- Lines 30-80 (with numbers via awk) ----"
awk 'NR>=30 && NR<=80 {printf "%4d:%s\n", NR, $0}' "$FILE"
echo "---- django_migrations occurrences (repo-local) ----"
rg -n "django_migrations" "$FILE" || true
echo "---- transaction.atomic occurrences (repo-local) ----"
rg -n "transaction\.atomic" "$FILE" || true
echo "---- CREATE TABLE occurrences (repo-local) ----"
rg -n "CREATE TABLE.*django_migrations" -S specifyweb || true
echo "---- Dependency hints (Django version) ----"
for f in $(fd -t f -g "pyproject.toml" . 2>/dev/null | head -n 20; fd -t f -g "requirements*.txt" . 2>/dev/null | head -n 20; fd -t f -g "Pipfile*" . 2>/dev/null | head -n 20); do
echo "---- $f ----"
rg -n "Django" "$f" || true
doneRepository: specify/specify7
Length of output: 3656
🌐 Web query:
Django 4.2 transaction.atomic caught exception set_rollback behavior
💡 Result:
In Django 4.2, the transaction.atomic context manager determines whether to commit or roll back based on how the block is exited [1][2]. If the block finishes normally, Django commits the transaction; if it exits due to an unhandled exception, Django rolls back the transaction [1][3]. When you catch an exception inside an atomic block, you prevent Django from seeing the error, which causes the transaction to be committed rather than rolled back [2][4]. This often leads to inconsistent database states because subsequent operations—which might rely on the failed operation—may proceed as if everything succeeded [3][4]. To handle exceptions correctly while ensuring a rollback occurs, you should use one of the following approaches: 1. Re-raise the exception: If you must catch an exception inside the block to perform logging or other tasks, you must re-raise the exception (e.g., using raise) so that Django’s atomic mechanism detects it and triggers the rollback [5][4]. 2. Use set_rollback: If you want to force a rollback without propagating an exception (i.e., you intend to handle the error silently and continue execution), you can use django.db.transaction.set_rollback(True) [6][3]. This manually flags the current atomic block to roll back when it exits [6][7]. Note that this is an advanced feature and should be used with caution to avoid data corruption or breaking the state of the atomic block [6][3]. 3. Catch outside the block: The recommended pattern is to catch exceptions outside the atomic block [3][2]. This clearly defines the scope of the transaction and prevents the confusion of handling exceptions within the same logic that manages the database state [3][2]. Example using set_rollback: from django.db import transaction @transaction.atomic def my_view: # Perform database operations if some_failure_condition: transaction.set_rollback(True) return False return True As of June 1, 2026, Django 4.2 has reached its end-of-life (April 7, 2026) [8]. It is recommended to upgrade to a supported version, such as Django 5.2 LTS, to receive ongoing security updates and support [8].
Citations:
- 1: https://django.readthedocs.io/en/4.1.x/topics/db/transactions.html
- 2: https://docs.djangoproject.com/en/dev/topics/db/transactions/?from=olddocs
- 3: https://docs.djangoproject.com/en/6.0/topics/db/transactions/
- 4: https://stackoverflow.com/questions/34730385/django-rollback-save-with-transaction-atomic
- 5: https://stackoverflow.com/questions/42586841/is-it-ok-to-catch-and-reraise-an-exception-inside-django-transaction-atomic
- 6: https://pypi-browser.org/package/django/Django-4.2.20-py3-none-any.whl/django/db/transaction.py
- 7: https://docs.djangoproject.com/en/5.0/_modules/django/db/transaction/
- 8: https://dev.to/endoflifeai/django-end-of-life-dates-official-eol-schedule-for-every-version-30dd
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for DB ENGINE configuration
rg -n "ENGINE\s*=" specifyweb | head -n 50
rg -n "\"ENGINE\"|ENGINE" specifyweb/settings -S 2>/dev/null | head -n 50 || true
rg -n "DATABASES" specifyweb -S | head -n 50
# Look for mysql/postgres driver usage
rg -n "mysqlclient|pymysql|psycopg|postgres|django\.db\.backends\.mysql|django\.db\.backends\.postgres" -S . | head -n 80
# Also show relevant portion of base_specify_migration.py around the atomic block using sed
FILE="specifyweb/specify/management/commands/base_specify_migration.py"
sed -n '20,100p' "$FILE"Repository: specify/specify7
Length of output: 4095
Adjust django_migrations probe error handling inside the MySQL transaction.atomic() block
In specifyweb/specify/management/commands/base_specify_migration.py (lines 38-53), the code catches the SELECT 1 FROM django_migrations failure with a bare except: and then continues inside transaction.atomic(using=alias). In Django 4.2, a caught exception won’t trigger the atomic rollback behavior, so the “broken transaction / must be rolled back” framing doesn’t match how atomic works when errors are swallowed.
More importantly, the bare except: can mask failures other than “table missing” (e.g., permissions/connection issues), which then makes the bootstrap proceed to CREATE TABLE IF NOT EXISTS and potentially hide the real underlying problem—especially since this project uses a MySQL backend (specifyweb.backend.hibernateboolsbackend.backends.mysql, mysqlclient).
Use a Django-native existence check (introspection) or catch only the specific “table doesn’t exist” error and re-raise everything else (or call set_rollback if you intentionally continue after a database error).
🧰 Tools
🪛 Ruff (0.15.15)
[error] 48-48: Do not use bare except
(E722)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@specifyweb/specify/management/commands/base_specify_migration.py` around
lines 38 - 53, The current probe around SELECT 1 FROM django_migrations inside
transaction.atomic(using=alias) swallows all exceptions via a bare except and
can hide non-“table missing” errors; change the logic in the block that uses
conn.cursor(), cursor.execute(...) and the exists flag to either use Django's DB
introspection (connection.introspection.table_names() or
connection.introspection.table_exists) to check for the django_migrations table,
or catch only the specific DB-API error that indicates “table does not exist”
(e.g. the MySQL error/exception type your backend raises) and re-raise any other
exceptions (or call transaction.set_rollback(True) before continuing) so you
don't mask permission/connection errors and so atomic(using=alias) behaves
correctly.
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
2 participants
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.
Automatic DB user creation for new Specify 7 instances
Fixes #6121
Checklist
self-explanatory (or properly documented)
specify7/specifyweb/specify/management/commands/run_key_migration_functions.py
Line 50 in ea04665
Testing instructions
Summary by CodeRabbit
New Features
Chores