Fix to use PG user from env variables - #593
Conversation
WalkthroughThe changes introduce a new Changes
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
codegenerator/cli/npm/envio/src/PgStorage.res (1)
55-63: Signature updated but not documented
makeInitializeTransactionnow requires~pgUser, yet the file header / doc-comments were not updated. Adding a one-liner explaining the new argument will prevent misuse.scenarios/test_codegen/test/lib_tests/EntityHistory_test.res (1)
240-243: Avoid magic strings forpgUserin tests
"postgres"is duplicated in several tests. Define once:let pgUser = "postgres"and reuse to keep refactors trivial.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
scenarios/test_codegen/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
codegenerator/cli/npm/envio/src/PgStorage.res(4 hunks)codegenerator/cli/templates/static/codegen/src/Config.res(1 hunks)scenarios/test_codegen/test/lib_tests/EntityHistory_test.res(4 hunks)scenarios/test_codegen/test/lib_tests/PgStorage_test.res(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build_and_test
🔇 Additional comments (3)
codegenerator/cli/templates/static/codegen/src/Config.res (1)
88-89: Parameter correctly propagated – looks goodPassing
~pgUser=Env.Db.userforwards the new mandatory parameter without hard-wiring the role. No further issues spotted here.codegenerator/cli/npm/envio/src/PgStorage.res (1)
151-168: API surface changed – consider version bump
makeis now a breaking change (new required param). Down-stream packages importing this module will fail to compile. Recommend bumping the minor version (or major if semver is strictly followed) and updating changelog.scenarios/test_codegen/test/lib_tests/PgStorage_test.res (1)
115-118: Expected SQL must follow quoting decisionIf the production code is updated to quote
pgUser, remember to mirror it here:-CREATE SCHEMA "test_schema";GRANT ALL ON SCHEMA "test_schema" TO postgres; +CREATE SCHEMA "test_schema";GRANT ALL ON SCHEMA "test_schema" TO "postgres";Failing to align tests will cause brittle failures.
| `GRANT ALL ON SCHEMA "${pgSchema}" TO ${pgUser}; | ||
| GRANT ALL ON SCHEMA "${pgSchema}" TO public;`, | ||
| ) |
There was a problem hiding this comment.
Quote pgUser to avoid SQL-injection & case-sensitivity pitfalls
pgUser is injected unquoted, which:
- Breaks when the role contains caps or special chars.
- Opens a minor injection vector if the env var is ever user-supplied.
A minimal, safe fix:
- `GRANT ALL ON SCHEMA "${pgSchema}" TO ${pgUser};
+ `GRANT ALL ON SCHEMA "${pgSchema}" TO "${pgUser}";📝 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.
| `GRANT ALL ON SCHEMA "${pgSchema}" TO ${pgUser}; | |
| GRANT ALL ON SCHEMA "${pgSchema}" TO public;`, | |
| ) | |
| `GRANT ALL ON SCHEMA "${pgSchema}" TO "${pgUser}"; | |
| GRANT ALL ON SCHEMA "${pgSchema}" TO public;`, | |
| ) |
🤖 Prompt for AI Agents
In codegenerator/cli/npm/envio/src/PgStorage.res around lines 79 to 81, the
variable pgUser is injected into the SQL string without quotes, which risks SQL
injection and breaks if the role contains uppercase letters or special
characters. Fix this by wrapping pgUser in double quotes within the SQL string
to ensure proper quoting and prevent injection vulnerabilities.
2 participants
Summary by CodeRabbit
New Features
Tests