Skip to content

Develop#1725

Open
abhishek8shankar wants to merge 2 commits into
mosip:developfrom
abhishek8shankar:develop
Open

Develop#1725
abhishek8shankar wants to merge 2 commits into
mosip:developfrom
abhishek8shankar:develop

Conversation

@abhishek8shankar
Copy link
Copy Markdown
Member

@abhishek8shankar abhishek8shankar commented May 13, 2026

Summary by CodeRabbit

  • New Features

    • Database scripts now support custom database and user names through parameterization, enabling flexible deployment configurations instead of hardcoded values.
  • Documentation

    • Updated README to reflect parameterized database script paths.
  • Chores

    • Updated deployment scripts and SQL files to use variable placeholders for database and user names, improving deployment flexibility and reusability across different environments.

Review Change Stack

abhishek8shankar and others added 2 commits May 11, 2026 15:52
Signed-off-by: Abhi <abhishek.shankarcs@gmail.com>
Signed-off-by: Abhishek S <127825992+abhishek8shankar@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

Walkthrough

This PR parameterizes MOSIP IDA database scripts to support multi-instance deployments. Database and role names are converted from hardcoded values to psql variable placeholders (:mosipdbname, :dbuname), allowing the same scripts to initialize databases with different names and configurations.

Changes

Database Script Parameterization

Layer / File(s) Summary
Configuration and Documentation
db_scripts/mosip_ida/deploy.properties, db_scripts/README.MD
Added DB_UNAME property and updated documentation examples to reflect parameterized database script folder paths from mosip_ida to :mosipdbname.
Core Database Initialization Scripts
db_scripts/mosip_ida/db.sql, ddl.sql, dml.sql, drop_db.sql
Database creation, schema setup, connection, and DML scripts transition from hardcoded mosip_ida to :mosipdbname variable references.
Table and Schema DDL Definitions
db_scripts/mosip_ida/ddl/ida-*.sql (16 files), ddl/ida-fk.sql
All table definition scripts update header comments to use :mosipdbname; ida-fk.sql additionally parameterizes GRANT statements to use :dbuname placeholder.
Role and Permission Management
db_scripts/mosip_ida/role_dbuser.sql, drop_role.sql, grants.sql
Role creation and privilege grant statements replace hardcoded role name idauser with :dbuname, and database name references with :mosipdbname, while preserving all privilege scopes.
Deployment Integration
db_scripts/mosip_ida/deploy.sh
Updated psql invocations to pass database name, username, and password variables via -v flags for all database setup, role creation, and grant operations.
Database Cloning Utility
db_scripts/createIdaNewDB.sh
Parameterized shell script clones existing database directories and applies sed replacements to update embedded hardcoded references to database and role names across SQL and properties files.

🎯 3 (Moderate) | ⏱️ ~25 minutes

🐰 Hop, hop! The scripts now dance with variables swift,
No more hardcoded names—a parameterized gift!
From ida to :mosipdbname, roles take new flight,
Clone any database instance, deploy day or night! 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Develop' is too vague and generic; it does not convey any meaningful information about the actual changeset involving database script parameterization. Replace the title with a descriptive summary of the main change, such as 'Parameterize database scripts for flexible database naming' or 'Add database name and username placeholders to MOSIP IDA scripts'.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
db_scripts/createIdaNewDB.sh (1)

3-3: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Usage message references the wrong script name.

Line 3 prints reNameDB.sh, but the file is createIdaNewDB.sh, which is confusing for operators.

Suggested fix
-echo "syntax: bash reNameDB.sh oldDbName newDbName password";
+echo "syntax: bash createIdaNewDB.sh oldDbName newDbName password"
🤖 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_scripts/createIdaNewDB.sh` at line 3, Update the usage message printed by
the script so it references the correct script name and parameters: replace the
incorrect "reNameDB.sh" text in the echo statement with "createIdaNewDB.sh" and
ensure the rest of the usage string still shows the expected arguments
(oldDbName newDbName password) so operators see the correct invocation for
createIdaNewDB.sh.
🧹 Nitpick comments (1)
db_scripts/createIdaNewDB.sh (1)

1-8: ⚡ Quick win

Fail fast on command errors to avoid partial clone state.

This script mutates files in-place across many steps; without strict mode, an early failure can still continue and leave a half-updated DB directory.

Suggested fix
 #!/usr/bin/env bash
+set -euo pipefail
🤖 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_scripts/createIdaNewDB.sh` around lines 1 - 8, Add Bash strict mode so the
script fails fast: immediately after the shebang (#! /usr/bin/env bash) enable
"set -euo pipefail" and set a safe IFS (e.g. IFS=$'\n\t') so any command
failure, unset variable, or pipeline error aborts the script and avoids leaving
a half-updated DB; keep the existing argument check (the if [[ $# -ne 3 ]]
block) but run it under the strict mode and consider adding a trap to cleanup on
exit if needed.
🤖 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_scripts/createIdaNewDB.sh`:
- Around line 20-26: The sed invocations inject raw $oldDB and $newDB which can
break patterns if they contain /, &, or backslashes; create escaped variables
(e.g., escaped_oldDB and escaped_newDB) by escaping backslashes, ampersands and
the chosen sed delimiter, reuse those escaped variables in all sed commands
(including the blocks at lines 20-26 and 31-37), and optionally switch sed to a
safe delimiter (e.g., |) so substitutions like sed -i
"s|$escaped_oldDB|$escaped_newDB|g" $newDB/:mosipdbname_deploy.properties are
robust.
- Around line 16-17: The rm -rf $newDB and cp -r $oldDB $newDB operations are
unsafe and unquoted; update the script to validate and quote the path variables
(newDB, oldDB) before use: ensure newDB and oldDB are non-empty, not "/" or "."
(or other dangerous values), and that oldDB exists and is a directory; only then
run the destructive commands, quoting expansions (e.g., use "$newDB" and
"$oldDB") and consider using safer rm flags or a targeted delete routine to
avoid accidental deletions.

In `@db_scripts/mosip_ida/deploy.sh`:
- Line 25: The psql invocations (e.g., the line setting PGPASSWORD and calling
psql) pass shell variables unquoted which breaks on special characters and
allows word-splitting/globbing; update each psql invocation (references:
PGPASSWORD, SU_USER_PWD, SU_USER, DB_SERVERIP, DB_PORT, DEFAULT_DB_NAME,
MOSIP_DB_NAME and the psql -f calls like drop_db.sql) to properly quote
expansions: export or set the PGPASSWORD value quoted, and wrap all variable
expansions in double quotes when used as arguments to psql (including
--username, --host, --port, --dbname and -v mosipdbname) and remove fragile
escaped-quote patterns so the commands are robust against spaces and special
chars.

In `@db_scripts/mosip_ida/drop_role.sql`:
- Line 1: The DROP ROLE statement interpolates the variable :dbuname as a raw
identifier which can break for non-simple names or allow malformed input; change
the interpolation to use the identifier-safe form by quoting the bind as
:"dbuname" in the DROP ROLE statement (the symbol to update is the DROP ROLE
line containing :dbuname) so psql treats it as an identifier and avoids invalid
SQL or injection issues.

In `@db_scripts/mosip_ida/grants.sql`:
- Line 1: The script uses raw psql variable substitution for identifiers (e.g.,
:mosipdbname and other role/database placeholders referenced on lines
1,4–5,9,13,16); replace each raw form with the psql identifier-quoting syntax
(e.g., :"mosipdbname") so identifiers are safely quoted when used as
database/role names in commands like \c, CREATE DATABASE, CREATE ROLE, GRANT,
and ALTER DEFAULT PRIVILEGES; update every occurrence of :<var> to :"<var>" for
those identifier variables to ensure correct, portable identifier handling.

In `@db_scripts/README.MD`:
- Line 17: Update the README example to clarify that :mosipdbname is a psql
variable used inside SQL scripts and not a filesystem directory; explicitly
state that the physical script directory is db_scripts/mosip_ida and that this
folder contains scripts intended to be run against the :mosipdbname database, so
readers understand the distinction between the runtime parameter (:mosipdbname)
and the on-disk location (db_scripts/mosip_ida).

---

Outside diff comments:
In `@db_scripts/createIdaNewDB.sh`:
- Line 3: Update the usage message printed by the script so it references the
correct script name and parameters: replace the incorrect "reNameDB.sh" text in
the echo statement with "createIdaNewDB.sh" and ensure the rest of the usage
string still shows the expected arguments (oldDbName newDbName password) so
operators see the correct invocation for createIdaNewDB.sh.

---

Nitpick comments:
In `@db_scripts/createIdaNewDB.sh`:
- Around line 1-8: Add Bash strict mode so the script fails fast: immediately
after the shebang (#! /usr/bin/env bash) enable "set -euo pipefail" and set a
safe IFS (e.g. IFS=$'\n\t') so any command failure, unset variable, or pipeline
error aborts the script and avoids leaving a half-updated DB; keep the existing
argument check (the if [[ $# -ne 3 ]] block) but run it under the strict mode
and consider adding a trap to cleanup on exit if needed.
🪄 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

Run ID: 90638dcf-b23d-4037-917a-f76eb22a74b1

📥 Commits

Reviewing files that changed from the base of the PR and between 6189562 and e58b7cf.

📒 Files selected for processing (28)
  • db_scripts/README.MD
  • db_scripts/createIdaNewDB.sh
  • db_scripts/mosip_ida/db.sql
  • db_scripts/mosip_ida/ddl.sql
  • db_scripts/mosip_ida/ddl/ida-anonymous_profile.sql
  • db_scripts/mosip_ida/ddl/ida-api_key_data.sql
  • db_scripts/mosip_ida/ddl/ida-auth_transaction.sql
  • db_scripts/mosip_ida/ddl/ida-ca_cert_store.sql
  • db_scripts/mosip_ida/ddl/ida-credential_event_store.sql
  • db_scripts/mosip_ida/ddl/ida-data_encrypt_keystore.sql
  • db_scripts/mosip_ida/ddl/ida-fk.sql
  • db_scripts/mosip_ida/ddl/ida-ident_binding_cert_store.sql
  • db_scripts/mosip_ida/ddl/ida-identity_cache.sql
  • db_scripts/mosip_ida/ddl/ida-key_store.sql
  • db_scripts/mosip_ida/ddl/ida-misp_license_data.sql
  • db_scripts/mosip_ida/ddl/ida-oidc_client_data.sql
  • db_scripts/mosip_ida/ddl/ida-otp_transaction.sql
  • db_scripts/mosip_ida/ddl/ida-partner_data.sql
  • db_scripts/mosip_ida/ddl/ida-partner_mapping.sql
  • db_scripts/mosip_ida/ddl/ida-uin_auth_lock.sql
  • db_scripts/mosip_ida/ddl/ida-uin_hash_salt.sql
  • db_scripts/mosip_ida/deploy.properties
  • db_scripts/mosip_ida/deploy.sh
  • db_scripts/mosip_ida/dml.sql
  • db_scripts/mosip_ida/drop_db.sql
  • db_scripts/mosip_ida/drop_role.sql
  • db_scripts/mosip_ida/grants.sql
  • db_scripts/mosip_ida/role_dbuser.sql

Comment on lines 16 to 17
rm -rf $newDB
cp -r $oldDB $newDB
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Guard and quote destructive filesystem operations.

Line 16 can delete unintended paths (/, ., empty-like values after expansion) and both lines perform word splitting/globbing due to unquoted vars. Add validation and quote all path expansions before rm/cp.

Suggested fix
+set -euo pipefail
+
+if [[ -z "${oldDB}" || -z "${newDB}" || "${newDB}" == "/" || "${newDB}" == "." ]]; then
+  echo "Refusing unsafe path values for oldDB/newDB; EXITING"
+  exit 1
+fi
+
-rm -rf $newDB
-cp -r $oldDB $newDB
+rm -rf -- "$newDB"
+cp -r -- "$oldDB" "$newDB"
🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 16-16: Double quote to prevent globbing and word splitting.

(SC2086)


[info] 17-17: Double quote to prevent globbing and word splitting.

(SC2086)


[info] 17-17: 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_scripts/createIdaNewDB.sh` around lines 16 - 17, The rm -rf $newDB and cp
-r $oldDB $newDB operations are unsafe and unquoted; update the script to
validate and quote the path variables (newDB, oldDB) before use: ensure newDB
and oldDB are non-empty, not "/" or "." (or other dangerous values), and that
oldDB exists and is a directory; only then run the destructive commands, quoting
expansions (e.g., use "$newDB" and "$oldDB") and consider using safer rm flags
or a targeted delete routine to avoid accidental deletions.

Comment on lines +20 to +26
sed -i "s/$oldDB\>/$newDB/g" $newDB/:mosipdbname_deploy.properties;
sed -i "s/$oldDB/$newDB/g" $newDB/:mosipdbname_db.sql;
sed -i "s/$oldDB/$newDB/g" $newDB/:mosipdbname_ddl_deploy.sql;
sed -i "s/$oldDB/$newDB/g" $newDB/:mosipdbname_dml_deploy.sql;
sed -i "s/$oldDB/$newDB/g" $newDB/:mosipdbname_grants.sql;
sed -i "s/$oldDB/$newDB/g" $newDB/mosip_role_common.sql;
sed -i "s/$oldDB/$newDB/g" $newDB/mosip_role_idauser.sql;
sed -i "s/$oldDB/$newDB/g" $newDB/mosip_role_:dbuname.sql;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Escape sed search/replacement inputs before interpolation.

Lines 20-26 and 31-37 inject raw variables into sed patterns/replacements. Values containing /, &, \, or regex chars can corrupt output or make replacements fail. Escape values once and reuse.

Suggested fix
+escape_sed_pattern() { printf '%s' "$1" | sed 's/[.[\*^$()+?{|\\]/\\&/g'; }
+escape_sed_repl() { printf '%s' "$1" | sed 's/[&/\\]/\\&/g'; }
+
+oldDB_pat="$(escape_sed_pattern "$oldDB")"
+newDB_repl="$(escape_sed_repl "$newDB")"
+dbPwd_repl="$(escape_sed_repl "$dbPwd")"
+
-sed -i "s/$oldDB\>/$newDB/g" $newDB/:mosipdbname_deploy.properties;
+sed -i "s/${oldDB_pat}\>/${newDB_repl}/g" "$newDB/:mosipdbname_deploy.properties"
...
-sed -i "s/SYSADMIN_PWD=.*/SYSADMIN_PWD=$dbPwd/g" $newDB/:mosipdbname_deploy.properties;
+sed -i "s/SYSADMIN_PWD=.*/SYSADMIN_PWD=${dbPwd_repl}/g" "$newDB/:mosipdbname_deploy.properties"

Also applies to: 31-37

🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 20-20: Double quote to prevent globbing and word splitting.

(SC2086)


[info] 21-21: Double quote to prevent globbing and word splitting.

(SC2086)


[info] 22-22: Double quote to prevent globbing and word splitting.

(SC2086)


[info] 23-23: Double quote to prevent globbing and word splitting.

(SC2086)


[info] 24-24: Double quote to prevent globbing and word splitting.

(SC2086)


[info] 25-25: Double quote to prevent globbing and word splitting.

(SC2086)


[info] 26-26: 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_scripts/createIdaNewDB.sh` around lines 20 - 26, The sed invocations
inject raw $oldDB and $newDB which can break patterns if they contain /, &, or
backslashes; create escaped variables (e.g., escaped_oldDB and escaped_newDB) by
escaping backslashes, ampersands and the chosen sed delimiter, reuse those
escaped variables in all sed commands (including the blocks at lines 20-26 and
31-37), and optionally switch sed to a safe delimiter (e.g., |) so substitutions
like sed -i "s|$escaped_oldDB|$escaped_newDB|g"
$newDB/:mosipdbname_deploy.properties are robust.

## Drop db and role
echo "Dropping DB"
PGPASSWORD=$SU_USER_PWD psql -v ON_ERROR_STOP=1 --username=$SU_USER --host=$DB_SERVERIP --port=$DB_PORT --dbname=$DEFAULT_DB_NAME -f drop_db.sql
PGPASSWORD=$SU_USER_PWD psql -v ON_ERROR_STOP=1 --username=$SU_USER --host=$DB_SERVERIP --port=$DB_PORT --dbname=$DEFAULT_DB_NAME -f drop_db.sql -v mosipdbname=$MOSIP_DB_NAME
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify unquoted variable expansions in changed psql invocation lines.
nl -ba db_scripts/mosip_ida/deploy.sh | sed -n '20,50p'
rg -nP 'psql .*--(username|host|port|dbname)=\$| -v [a-zA-Z_]+=\\?\$' db_scripts/mosip_ida/deploy.sh

Repository: mosip/id-authentication

Length of output: 1722


Quote all shell expansions in psql calls; current form breaks on special characters.

Multiple psql invocations pass unquoted variables to arguments and the PGPASSWORD environment variable, risking word splitting and globbing failures. Line 32 uses a fragile escaped-quote pattern that is error-prone.

Suggested fix (pattern)
-PGPASSWORD=$SU_USER_PWD psql -v ON_ERROR_STOP=1 --username=$SU_USER --host=$DB_SERVERIP --port=$DB_PORT --dbname=$DEFAULT_DB_NAME -f drop_db.sql -v mosipdbname=$MOSIP_DB_NAME
+PGPASSWORD="$SU_USER_PWD" psql -v ON_ERROR_STOP=1 --username="$SU_USER" --host="$DB_SERVERIP" --port="$DB_PORT" --dbname="$DEFAULT_DB_NAME" -f drop_db.sql -v "mosipdbname=$MOSIP_DB_NAME"

-PGPASSWORD=$SU_USER_PWD psql -v ON_ERROR_STOP=1 --username=$SU_USER --host=$DB_SERVERIP --port=$DB_PORT --dbname=$DEFAULT_DB_NAME -f role_dbuser.sql -v dbuserpwd=\'$DBUSER_PWD\' -v dbuname=$DB_UNAME
+PGPASSWORD="$SU_USER_PWD" psql -v ON_ERROR_STOP=1 --username="$SU_USER" --host="$DB_SERVERIP" --port="$DB_PORT" --dbname="$DEFAULT_DB_NAME" -f role_dbuser.sql -v "dbuserpwd=$DBUSER_PWD" -v "dbuname=$DB_UNAME"

Also applies to: 28-28, 36-36, 37-37, 40-40, 46-46

🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 25-25: Double quote to prevent globbing and word splitting.

(SC2086)


[info] 25-25: Double quote to prevent globbing and word splitting.

(SC2086)


[info] 25-25: Double quote to prevent globbing and word splitting.

(SC2086)


[info] 25-25: Double quote to prevent globbing and word splitting.

(SC2086)


[info] 25-25: 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_scripts/mosip_ida/deploy.sh` at line 25, The psql invocations (e.g., the
line setting PGPASSWORD and calling psql) pass shell variables unquoted which
breaks on special characters and allows word-splitting/globbing; update each
psql invocation (references: PGPASSWORD, SU_USER_PWD, SU_USER, DB_SERVERIP,
DB_PORT, DEFAULT_DB_NAME, MOSIP_DB_NAME and the psql -f calls like drop_db.sql)
to properly quote expansions: export or set the PGPASSWORD value quoted, and
wrap all variable expansions in double quotes when used as arguments to psql
(including --username, --host, --port, --dbname and -v mosipdbname) and remove
fragile escaped-quote patterns so the commands are robust against spaces and
special chars.

@@ -1 +1 @@
drop role if exists idauser;
drop role if exists :dbuname;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify identifier interpolation style for db/user placeholders in SQL scripts.
rg -nP '(:dbuname|:mosipdbname)\b' db_scripts/mosip_ida/*.sql db_scripts/mosip_ida/ddl/*.sql
rg -nP ':"(dbuname|mosipdbname)"' db_scripts/mosip_ida/*.sql db_scripts/mosip_ida/ddl/*.sql

Repository: mosip/id-authentication

Length of output: 2590


Use identifier-safe psql interpolation for role name.

Line 1 interpolates :dbuname raw in identifier context. Use :"dbuname" to avoid invalid SQL for non-simple names and reduce injection risk from malformed values.

Suggested fix
-drop role if exists :dbuname;
+drop role if exists :"dbuname";
📝 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
drop role if exists :dbuname;
drop role if exists :"dbuname";
🤖 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_scripts/mosip_ida/drop_role.sql` at line 1, The DROP ROLE statement
interpolates the variable :dbuname as a raw identifier which can break for
non-simple names or allow malformed input; change the interpolation to use the
identifier-safe form by quoting the bind as :"dbuname" in the DROP ROLE
statement (the symbol to update is the DROP ROLE line containing :dbuname) so
psql treats it as an identifier and avoids invalid SQL or injection issues.

@@ -1,17 +1,17 @@
\c mosip_ida
\c :mosipdbname
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

❓ Verification inconclusive

Script executed:

find . -name "grants.sql" -path "*/mosip_ida/*" -type f

Repository: mosip/id-authentication


Repository: mosip/id-authentication
Exit code: 0

stdout:

./db_scripts/mosip_ida/grants.sql


Script executed:

cat -n db_scripts/mosip_ida/grants.sql | head -30

Repository: mosip/id-authentication


Repository: mosip/id-authentication
Exit code: 0

stdout:

     1	\c :mosipdbname
     2	
     3	GRANT CONNECT
     4	   ON DATABASE :mosipdbname
     5	   TO :dbuname;
     6	
     7	GRANT USAGE
     8	   ON SCHEMA ida
     9	   TO :dbuname;
    10	
    11	GRANT SELECT,INSERT,UPDATE,DELETE,TRUNCATE,REFERENCES
    12	   ON ALL TABLES IN SCHEMA ida
    13	   TO :dbuname;
    14	
    15	ALTER DEFAULT PRIVILEGES IN SCHEMA ida 
    16		GRANT SELECT,INSERT,UPDATE,DELETE,REFERENCES ON TABLES TO :dbuname;
    17	


psql identifier quoting :"variable" interpolation best practices


In psql, use variable interpolation with specific quoting syntax for identifiers and literals to ensure safe substitution. For identifiers (table/column names needing double quotes), use:"varname" which applies proper double-quoting [1][2][3][4]. For SQL literals (string values), use:'varname' which applies single-quoting [1][5][3]. Example: \set table_name 'MyTable' \set col_name 'UserId' SELECT * FROM:"table_name" WHERE:"col_name" = 123; -- becomes SELECT * FROM "MyTable" WHERE "UserId" = 123; [3] This handles case sensitivity, keywords, and special characters automatically, equivalent to quote_ident and quote_literal [6][4]. Unquoted:varname substitutes raw value, suitable only for simple identifiers without specials [1]. Best practices: - Always use:"var" for identifiers to avoid parse errors with keywords/case [6][7][3]. - Use:'var' for values to prevent SQL injection-like issues [5][8]. - Avoid mixed-case or keyword identifiers; use lowercase unquoted for portability [6][9][10]. - Set variables with \set var 'value' or psql -v var=value [5][3]. - Interpolation is syntax-aware and skips strings/blocks [11][3]. For \f (field separator), it sets output formatting, unrelated to identifier quoting [1].

Citations:


Use identifier-quoted psql variables consistently in grants script.

Raw :var substitution is unsafe for identifiers that may contain mixed case, reserved keywords, or special characters. Apply the psql identifier-quoting syntax :"var" to all database and role name placeholders (lines 1, 4–5, 9, 13, 16) for robust, portable execution aligned with PostgreSQL best practices.

Suggested fix
-\c :mosipdbname
+\c :"mosipdbname"
-   ON DATABASE :mosipdbname
-   TO :dbuname;
+   ON DATABASE :"mosipdbname"
+   TO :"dbuname";
-   TO :dbuname;
+   TO :"dbuname";
-   TO :dbuname;
+   TO :"dbuname";
-	GRANT SELECT,INSERT,UPDATE,DELETE,REFERENCES ON TABLES TO :dbuname;
+	GRANT SELECT,INSERT,UPDATE,DELETE,REFERENCES ON TABLES TO :"dbuname";
📝 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
\c :mosipdbname
\c :"mosipdbname"
🤖 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_scripts/mosip_ida/grants.sql` at line 1, The script uses raw psql variable
substitution for identifiers (e.g., :mosipdbname and other role/database
placeholders referenced on lines 1,4–5,9,13,16); replace each raw form with the
psql identifier-quoting syntax (e.g., :"mosipdbname") so identifiers are safely
quoted when used as database/role names in commands like \c, CREATE DATABASE,
CREATE ROLE, GRANT, and ALTER DEFAULT PRIVILEGES; update every occurrence of
:<var> to :"<var>" for those identifier variables to ensure correct, portable
identifier handling.

Comment thread db_scripts/README.MD
* Database objects related to MOSIP modules are placed in "**mosip_base_directory**>>db_scripts>>mosip_<schema_name> folder on git/repository

**Example:** the id-authentication module script folder is /**mosip_base_directory**>>db_scripts>>mosip_ida where all the database scripts related to id authentication are available.
**Example:** the id-authentication module script folder is /**mosip_base_directory**>>db_scripts>>:mosipdbname where all the database scripts related to id authentication are available.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clarify the distinction between psql variables and filesystem paths.

The example path uses :mosipdbname as if it were an actual directory name, but this is a psql variable placeholder used within SQL scripts, not a filesystem directory. The actual directory on disk is still db_scripts/mosip_ida. This creates confusion between the parameterized database name (used at runtime) and the physical script location.

Consider revising to clarify: "the id-authentication module script folder is db_scripts/mosip_ida where scripts for the :mosipdbname database are available."

🤖 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_scripts/README.MD` at line 17, Update the README example to clarify that
:mosipdbname is a psql variable used inside SQL scripts and not a filesystem
directory; explicitly state that the physical script directory is
db_scripts/mosip_ida and that this folder contains scripts intended to be run
against the :mosipdbname database, so readers understand the distinction between
the runtime parameter (:mosipdbname) and the on-disk location
(db_scripts/mosip_ida).

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.

1 participant