-
Notifications
You must be signed in to change notification settings - Fork 70
Ensure table and column names escaped #5850
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5850 +/- ##
==========================================
- Coverage 76.56% 76.49% -0.08%
==========================================
Files 369 369
Lines 17951 17969 +18
Branches 4179 4189 +10
==========================================
Hits 13745 13745
- Misses 4206 4224 +18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is fine as it (minus the lint errors), but I have concerns regarding other parts of the original merged PR I think we must handle to mitigate what we can
NOTE: the below is applicable to both drivers
1
this should be whitelisted at best, escaped at minimum
2
https://github.com/FlowFuse/flowfuse/pull/5850/files#diff-41c5997c75a79615b0da0be7da5a91b8daea9331ccf48714137091ebf76e5562R251-R255
col.default is user input and should be vetted for length and type and if string, escaped. Something like:
if (typeof col.default === 'string') {
const escapedDefault = pgLib.pg.escapeLiteral(col.default)
columnDef += ` DEFAULT ${escapedDefault}`
} else if (typeof col.default === 'number' || typeof col.default === 'boolean') {
columnDef += ` DEFAULT ${col.default}`
} else {
throw new Error(`Unsupported default value type for column ${col.name}`)
}
3
There are no unit tests for the createTable
or dropTable
functions.
coerce default values
Description
Enforce escaping for table and column names