BDMS-471-472-473 #404
BDMS-471-472-473 #404
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
This PR addresses SQL injection vulnerabilities in the database initialization code by properly quoting role names when granting permissions.
Changes:
- Added double-quote wrapping for role names in the GRANT statement to prevent SQL injection
- Introduced a
quotedvariable to store the properly formatted role name
| return | ||
| for member in members: | ||
| safe_member = member.replace("'", "''") | ||
| quoted = f'"{safe_member}"' |
There was a problem hiding this comment.
The quoting implementation is incomplete. While double quotes are added, the variable is used inside a single-quoted SQL string, meaning the double quotes will be treated as literal characters rather than SQL delimiters. The quoted variable should be used in a formatted part of the SQL string (outside the single quotes), or the string interpolation should be restructured to properly insert the quoted identifier.
| BEGIN | ||
| IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = '{safe_member}') THEN | ||
| EXECUTE 'GRANT app_read TO {safe_member}'; | ||
| EXECUTE 'GRANT app_read TO {quoted}'; |
There was a problem hiding this comment.
The {quoted} placeholder is inside a single-quoted string and will not be interpolated. It will be treated as the literal text '{quoted}' in the SQL command. This needs to be part of a formatted string (f-string) or use proper concatenation to insert the actual value of the quoted variable.
Why
This PR addresses the following problem / context:
How
Implementation summary - the following was changed / added / removed:
Notes
Any special considerations, workarounds, or follow-up work to note?