1899 Updated db_upgrade scripts for regproc - #2379
abhishek8shankar wants to merge 1 commit into
Conversation
Signed-off-by: abhishek8shankar <abhishek.shankarcs@gmail.com>
WalkthroughDatabase upgrade and rollback SQL scripts for mosip_regprc are changed to use psql variables ChangesDatabase Upgrade Script Parameterization
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@db_upgrade_scripts/mosip_regprc/upgrade.sh`:
- Line 24: The `CONN` assignment in `upgrade.sh` is unused and the trailing
`;exit;` inside the command substitution is redundant. Update the `CONN=$(...)`
shell block to remove the dead `exit` and either consume `CONN` later or drop
the assignment entirely if the value is not needed, keeping the `psql`
invocation in the same place for `MOSIP_DB_NAME`, `SU_USER`, and `DB_PORT`
handling.
- Line 24: The psql command in upgrade.sh is still vulnerable to word-splitting
because the newly added mosipdbname and dbuname variable expansions are
unquoted. Update the command in the upgrade script so the values passed via the
psql -v flags use quoted expansions for MOSIP_DB_NAME and DB_UNAME, keeping the
rest of the command structure in place and preserving the existing identifiers
psql, MOSIP_DB_NAME, and DB_UNAME.
- Line 33: Quote the newly added psql variable assignments in the upgrade and
rollback invocations to satisfy SC2086: in the command using psql, make sure the
-v mosipdbname and -v dbuname expansions are passed as quoted values, just like
the other shell variables in upgrade.sh. Update the matching rollback call as
well so both paths use the same quoted handling around MOSIP_DB_NAME and
DB_UNAME.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e65a3257-4467-4518-9dc8-37f8c7b5904e
📒 Files selected for processing (10)
db_upgrade_scripts/mosip_regprc/sql/1.1.5.5_to_1.2.0.1-B1_rollback.sqldb_upgrade_scripts/mosip_regprc/sql/1.1.5.5_to_1.2.0.1-B1_upgrade.sqldb_upgrade_scripts/mosip_regprc/sql/1.2.1.1_to_1.2.1.2_rollback.sqldb_upgrade_scripts/mosip_regprc/sql/1.2.1.1_to_1.2.1.2_upgrade.sqldb_upgrade_scripts/mosip_regprc/sql/1.2.1.2_to_1.3.0_rollback.sqldb_upgrade_scripts/mosip_regprc/sql/1.2.1.2_to_1.3.0_upgrade.sqldb_upgrade_scripts/mosip_regprc/sql/1.3.0_to_1.3.1_rollback.sqldb_upgrade_scripts/mosip_regprc/sql/1.3.0_to_1.3.1_upgrade.sqldb_upgrade_scripts/mosip_regprc/upgrade.propertiesdb_upgrade_scripts/mosip_regprc/upgrade.sh
| # Terminate existing connections | ||
| echo "Terminating active connections" | ||
| CONN=$(PGPASSWORD=$SU_USER_PWD psql -v ON_ERROR_STOP=1 --username=$SU_USER --host=$DB_SERVERIP --port=$DB_PORT --dbname=$DEFAULT_DB_NAME -t -c "SELECT count(pg_terminate_backend(pg_stat_activity.pid)) FROM pg_stat_activity WHERE datname = '$MOSIP_DB_NAME' AND pid <> pg_backend_pid()";exit;) | ||
| CONN=$(PGPASSWORD=$SU_USER_PWD psql -v ON_ERROR_STOP=1 -v mosipdbname=$MOSIP_DB_NAME -v dbuname=$DB_UNAME --username=$SU_USER --host=$DB_SERVERIP --port=$DB_PORT --dbname=$DEFAULT_DB_NAME -t -c "SELECT count(pg_terminate_backend(pg_stat_activity.pid)) FROM pg_stat_activity WHERE datname = '$MOSIP_DB_NAME' AND pid <> pg_backend_pid()";exit;) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
CONN is assigned but never used; trailing ;exit; inside the substitution is dead/confusing.
Shellcheck flags CONN as unused (SC2034). Also the ;exit; inside $(...) is redundant — it terminates the subshell but has no effect beyond normal subshell exit, and the result is never consumed anyway. Worth cleaning up while touching this line.
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 24-24: CONN appears unused. Verify use (or export if used externally).
(SC2034)
[info] 24-24: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 24-24: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 24-24: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 24-24: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 24-24: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 24-24: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 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 `@db_upgrade_scripts/mosip_regprc/upgrade.sh` at line 24, The `CONN` assignment
in `upgrade.sh` is unused and the trailing `;exit;` inside the command
substitution is redundant. Update the `CONN=$(...)` shell block to remove the
dead `exit` and either consume `CONN` later or drop the assignment entirely if
the value is not needed, keeping the `psql` invocation in the same place for
`MOSIP_DB_NAME`, `SU_USER`, and `DB_PORT` handling.
Source: Linters/SAST tools
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Quote the new $MOSIP_DB_NAME/$DB_UNAME expansions.
Static analysis flags unquoted expansions (SC2086) on this line, including the newly-added -v mosipdbname=$MOSIP_DB_NAME -v dbuname=$DB_UNAME. If either value ever contains whitespace, word-splitting will break the -v argument.
🛡️ Proposed fix
-CONN=$(PGPASSWORD=$SU_USER_PWD psql -v ON_ERROR_STOP=1 -v mosipdbname=$MOSIP_DB_NAME -v dbuname=$DB_UNAME --username=$SU_USER --host=$DB_SERVERIP --port=$DB_PORT --dbname=$DEFAULT_DB_NAME -t -c "SELECT count(pg_terminate_backend(pg_stat_activity.pid)) FROM pg_stat_activity WHERE datname = '$MOSIP_DB_NAME' AND pid <> pg_backend_pid()";exit;)
+CONN=$(PGPASSWORD="$SU_USER_PWD" psql -v ON_ERROR_STOP=1 -v mosipdbname="$MOSIP_DB_NAME" -v dbuname="$DB_UNAME" --username="$SU_USER" --host="$DB_SERVERIP" --port="$DB_PORT" --dbname="$DEFAULT_DB_NAME" -t -c "SELECT count(pg_terminate_backend(pg_stat_activity.pid)) FROM pg_stat_activity WHERE datname = '$MOSIP_DB_NAME' AND pid <> pg_backend_pid()")📝 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.
| CONN=$(PGPASSWORD=$SU_USER_PWD psql -v ON_ERROR_STOP=1 -v mosipdbname=$MOSIP_DB_NAME -v dbuname=$DB_UNAME --username=$SU_USER --host=$DB_SERVERIP --port=$DB_PORT --dbname=$DEFAULT_DB_NAME -t -c "SELECT count(pg_terminate_backend(pg_stat_activity.pid)) FROM pg_stat_activity WHERE datname = '$MOSIP_DB_NAME' AND pid <> pg_backend_pid()";exit;) | |
| CONN=$(PGPASSWORD="$SU_USER_PWD" psql -v ON_ERROR_STOP=1 -v mosipdbname="$MOSIP_DB_NAME" -v dbuname="$DB_UNAME" --username="$SU_USER" --host="$DB_SERVERIP" --port="$DB_PORT" --dbname="$DEFAULT_DB_NAME" -t -c "SELECT count(pg_terminate_backend(pg_stat_activity.pid)) FROM pg_stat_activity WHERE datname = '$MOSIP_DB_NAME' AND pid <> pg_backend_pid()") |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 24-24: CONN appears unused. Verify use (or export if used externally).
(SC2034)
[info] 24-24: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 24-24: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 24-24: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 24-24: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 24-24: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 24-24: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 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 `@db_upgrade_scripts/mosip_regprc/upgrade.sh` at line 24, The psql command in
upgrade.sh is still vulnerable to word-splitting because the newly added
mosipdbname and dbuname variable expansions are unquoted. Update the command in
the upgrade script so the values passed via the psql -v flags use quoted
expansions for MOSIP_DB_NAME and DB_UNAME, keeping the rest of the command
structure in place and preserving the existing identifiers psql, MOSIP_DB_NAME,
and DB_UNAME.
Source: Linters/SAST tools
| if [ -f "$UPGRADE_SCRIPT_FILE" ]; then | ||
| echo "Executing upgrade script $UPGRADE_SCRIPT_FILE" | ||
| PGPASSWORD=$SU_USER_PWD psql -v ON_ERROR_STOP=1 --username=$SU_USER --host=$DB_SERVERIP --port=$DB_PORT --dbname=$DEFAULT_DB_NAME -a -b -f $UPGRADE_SCRIPT_FILE | ||
| PGPASSWORD=$SU_USER_PWD psql -v ON_ERROR_STOP=1 -v mosipdbname=$MOSIP_DB_NAME -v dbuname=$DB_UNAME --username=$SU_USER --host=$DB_SERVERIP --port=$DB_PORT --dbname=$DEFAULT_DB_NAME -a -b -f $UPGRADE_SCRIPT_FILE |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Quote the new -v variable expansions here too.
Same SC2086 concern applies to the newly-added -v mosipdbname=$MOSIP_DB_NAME -v dbuname=$DB_UNAME on the upgrade/rollback invocations.
Also applies to: 43-43
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 33-33: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 33-33: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 33-33: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 33-33: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 33-33: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 33-33: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 33-33: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 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 `@db_upgrade_scripts/mosip_regprc/upgrade.sh` at line 33, Quote the newly added
psql variable assignments in the upgrade and rollback invocations to satisfy
SC2086: in the command using psql, make sure the -v mosipdbname and -v dbuname
expansions are passed as quoted values, just like the other shell variables in
upgrade.sh. Update the matching rollback call as well so both paths use the same
quoted handling around MOSIP_DB_NAME and DB_UNAME.
Source: Linters/SAST tools
1 participant
Summary by CodeRabbit