Skip to content

fix(cli): validate migration name and correctly report created migration files#4103

Merged
joanagmaia merged 3 commits into
linuxfoundation:mainfrom
Muawiya-contact:fix/migrations-cli
May 14, 2026
Merged

fix(cli): validate migration name and correctly report created migration files#4103
joanagmaia merged 3 commits into
linuxfoundation:mainfrom
Muawiya-contact:fix/migrations-cli

Conversation

@Muawiya-contact
Copy link
Copy Markdown
Contributor

@Muawiya-contact Muawiya-contact commented May 11, 2026

Problem

Two bugs in scripts/cli migration helpers:

  1. $MIG_FILE is printed in final output but never defined
  2. No validation when migration name argument is empty,
    silently creates broken files like V123456__ .sql
  3. File paths not quoted in touch calls

Fix

  • Added empty name validation with error + exit in both functions
  • Quoted $UP_MIG_FILE and $DOWN_MIG_FILE in touch calls
  • Fixed output message to show actual created filenames

Files Changed

  • scripts/cli

Closes #4102


Note

Low Risk
Low risk: small bash-script hardening for migration file creation (input validation, quoting, and log output) with no runtime impact beyond rejecting invalid names.

Overview
Tightens the scripts/cli migration helpers by trimming/normalizing the provided migration name, rejecting empty/invalid names, and exiting with a clear error.

Fixes migration file creation by quoting touch paths and updating the output to print the actual UP_MIG_FILE/DOWN_MIG_FILE names instead of an undefined variable.

Reviewed by Cursor Bugbot for commit 7fa0b52. Bugbot is set up for automated code reviews on this repo. Configure here.

Copilot AI review requested due to automatic review settings May 11, 2026 19:46
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes issues in the scripts/cli migration helper commands by adding basic argument validation and correcting the final “created” output so it reports the actual migration files created.

Changes:

  • Add required-argument validation for migration name in create_migration() and create_product_migration().
  • Quote migration file paths in touch calls to handle paths safely.
  • Fix the final output message to reference $UP_MIG_FILE and $DOWN_MIG_FILE (instead of undefined $MIG_FILE).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/cli Outdated
Comment thread scripts/cli Outdated
Comment thread scripts/cli Outdated
…ion files

Signed-off-by: Muawiya-contact <contactmuawia@gmail.com>
Copilot AI review requested due to automatic review settings May 12, 2026 01:34
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment thread scripts/cli
Comment thread scripts/cli
@Muawiya-contact
Copy link
Copy Markdown
Contributor Author

Hi @gaspergrom, @joanagmaia, @themarolt 👋

Just a friendly ping — this PR fixes two bugs in the migration CLI helpers (undefined variable output + missing input validation). It's a low-risk, single-file shell script change.

Would love to get your eyes on it when you have a moment. Happy to make any adjustments if needed!

Thanks 🙏

@themarolt themarolt self-requested a review May 13, 2026 17:56
@themarolt
Copy link
Copy Markdown
Contributor

@Muawiya-contact - the change looks ok - it would fail anyway later on with flyway but it's good that it can be prevented here as well - please merge main branch so it's ok to merge. cc @joanagmaia

@Muawiya-contact
Copy link
Copy Markdown
Contributor Author

@themarolt thanks for your honest feedback, I have merged the main branch on it. Can you take a look at it, #4075 please?
Thanks 🫶

Copilot AI review requested due to automatic review settings May 14, 2026 10:35
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

scripts/cli:312

  • Same issue as create_migration(): xargs will strip quotes/backslashes during parsing, so an invalid migration name may be altered into a valid one and pass validation. Prefer trimming with Bash string ops (or sed) without shell-style parsing semantics, then validate.
    MIG_NAME="$(printf '%s\n' "$1" | tr -s ' ' | xargs)"
    if [[ -z "$MIG_NAME" ]] || [[ ! "$MIG_NAME" =~ ^[A-Za-z0-9_-]+$ ]]; then

Comment thread scripts/cli
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit 7fa0b52. Configure here.

Comment thread scripts/cli
@joanagmaia joanagmaia merged commit 931ba4f into linuxfoundation:main May 14, 2026
9 of 10 checks passed
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.

fix(cli): Migration helper prints undefined variable and missing input validation

4 participants