Skip to content

Commit eb42a19

Browse files
soedirgosoedirgo
committed
fix: escaping identifiers & literals
We now use pg-format in most places, which should take care of escaping ' in COMMENTs.
1 parent c538242 commit eb42a19

File tree

9 files changed

+127
-53
lines changed

9 files changed

+127
-53
lines changed

‎package-lock.json‎

Lines changed: 12 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
"mocha": "^7.1.2",
3939
"nodemon": "^1.19.4",
4040
"npm-run-all": "^4.1.5",
41+
"pg-format": "^1.0.4",
4142
"pkg": "^4.4.8",
4243
"prettier": "^2.0.5",
4344
"rimraf": "^3.0.2",

‎src/api/columns.ts‎

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Router } from 'express'
2+
import format from 'pg-format'
23
import SQL from 'sql-template-strings'
34
import { RunQuery } from '../lib/connectionPool'
45
import sql = require('../lib/sql')
@@ -126,17 +127,22 @@ const addColumnSqlize = ({
126127
const commentSql =
127128
comment === undefined
128129
? ''
129-
: `COMMENT ON COLUMN "${schema}"."${table}"."${name}" IS '${comment}';`
130+
: format('COMMENT ON COLUMN %I.%I.%I IS %L;', schema, table, name, comment)
130131

131-
return `
132-
ALTER TABLE "${schema}"."${table}" ADD COLUMN "${name}" "${type}"
132+
return format(
133+
`
134+
ALTER TABLE %I.%I ADD COLUMN %I %I
133135
${defaultValueSql}
134136
${isIdentitySql}
135137
${isNullableSql}
136138
${isPrimaryKeySql}
137139
${isUniqueSql};
138-
139-
${commentSql}`
140+
${commentSql}`,
141+
schema,
142+
table,
143+
name,
144+
type
145+
)
140146
}
141147
const getColumnSqlize = (tableId: number, name: string) => {
142148
return SQL``.append(columns).append(SQL` WHERE c.oid = ${tableId} AND column_name = ${name}`)
@@ -168,30 +174,48 @@ const alterColumnSqlize = ({
168174
comment?: string
169175
}) => {
170176
const nameSql =
171-
typeof name === 'undefined' || name === oldName
177+
name === undefined || name === oldName
172178
? ''
173-
: `ALTER TABLE "${schema}"."${table}" RENAME COLUMN "${oldName}" TO "${name}";`
179+
: format('ALTER TABLE %I.%I RENAME COLUMN %I TO %I;', schema, table, oldName, name)
174180
// We use USING to allow implicit conversion of incompatible types (e.g. int4 -> text).
175181
const typeSql =
176182
type === undefined
177183
? ''
178-
: `ALTER TABLE "${schema}"."${table}" ALTER COLUMN "${oldName}" SET DATA TYPE "${type}" USING "${oldName}"::"${type}";`
184+
: format(
185+
'ALTER TABLE %I.%I ALTER COLUMN %I SET DATA TYPE %I USING %I::%I;',
186+
schema,
187+
table,
188+
oldName,
189+
type,
190+
oldName,
191+
type
192+
)
179193
let defaultValueSql = ''
180194
if (drop_default) {
181-
defaultValueSql = `ALTER TABLE "${schema}"."${table}" ALTER COLUMN "${oldName}" DROP DEFAULT;`
195+
defaultValueSql = format(
196+
'ALTER TABLE %I.%I ALTER COLUMN %I DROP DEFAULT;',
197+
schema,
198+
table,
199+
oldName
200+
)
182201
} else if (default_value !== undefined) {
183-
defaultValueSql = `ALTER TABLE "${schema}"."${table}" ALTER COLUMN "${oldName}" SET DEFAULT ${default_value};`
202+
defaultValueSql = format(
203+
`ALTER TABLE %I.%I ALTER COLUMN %I SET DEFAULT ${default_value};`,
204+
schema,
205+
table,
206+
oldName
207+
)
184208
}
185209
let isNullableSql = ''
186210
if (is_nullable !== undefined) {
187211
isNullableSql = is_nullable
188-
? `ALTER TABLE "${schema}"."${table}" ALTER COLUMN "${oldName}" DROP NOT NULL;`
189-
: `ALTER TABLE "${schema}"."${table}" ALTER COLUMN "${oldName}" SET NOT NULL;`
212+
? format('ALTER TABLE %I.%I ALTER COLUMN %I DROP NOT NULL;', schema, table, oldName)
213+
: format('ALTER TABLE %I.%I ALTER COLUMN %I SET NOT NULL;', schema, table, oldName)
190214
}
191215
const commentSql =
192216
comment === undefined
193217
? ''
194-
: `COMMENT ON COLUMN "${schema}"."${table}"."${oldName}" IS '${comment}';`
218+
: format('COMMENT ON COLUMN %I.%I.%I IS %L;', schema, table, oldName, comment)
195219

196220
// nameSql must be last.
197221
return `
@@ -204,7 +228,7 @@ BEGIN;
204228
COMMIT;`
205229
}
206230
const dropColumnSqlize = (schema: string, table: string, name: string) => {
207-
return `ALTER TABLE "${schema}"."${table}" DROP COLUMN "${name}"`
231+
return format('ALTER TABLE %I.%I DROP COLUMN %I', schema, table, name)
208232
}
209233
const removeSystemSchemas = (data: Tables.Column[]) => {
210234
return data.filter((x) => !DEFAULT_SYSTEM_SCHEMAS.includes(x.schema))

‎src/api/extensions.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Router } from 'express'
2+
import format from 'pg-format'
23
import SQL from 'sql-template-strings'
34
import sqlTemplates = require('../lib/sql')
45
const { extensions } = sqlTemplates
@@ -83,9 +84,9 @@ const createExtensionSqlize = ({
8384
cascade?: boolean
8485
}) => {
8586
return `
86-
CREATE EXTENSION "${name}"
87-
${schema === undefined ? '' : `SCHEMA ${schema}`}
88-
${version === undefined ? '' : `VERSION '${version}'`}
87+
CREATE EXTENSION ${format.ident(name)}
88+
${schema === undefined ? '' : `SCHEMA ${format.ident(schema)}`}
89+
${version === undefined ? '' : `VERSION ${format.literal(version)}`}
8990
${cascade ? 'CASCADE' : ''}`
9091
}
9192
const singleExtensionSqlize = (extensions: string, name: string) => {
@@ -104,9 +105,12 @@ const alterExtensionSqlize = ({
104105
}) => {
105106
let updateSql = ''
106107
if (update) {
107-
updateSql = `ALTER EXTENSION "${name}" UPDATE ${version === undefined ? '' : version};`
108+
updateSql = `ALTER EXTENSION ${format.ident(name)} UPDATE ${
109+
version === undefined ? '' : `TO ${format.literal(version)}`
110+
};`
108111
}
109-
const schemaSql = schema === undefined ? '' : `ALTER EXTENSION "${name}" SET SCHEMA "${schema}";`
112+
const schemaSql =
113+
schema === undefined ? '' : format('ALTER EXTENSION %I SET SCHEMA %I;', name, schema)
110114

111115
return `
112116
BEGIN;
@@ -116,7 +120,7 @@ COMMIT;`
116120
}
117121
const dropExtensionSqlize = (name: string, cascade: boolean) => {
118122
return `
119-
DROP EXTENSION ${name}
123+
DROP EXTENSION ${format.ident(name)}
120124
${cascade ? 'CASCADE' : 'RESTRICT'}`
121125
}
122126

‎src/api/policies.ts‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Router } from 'express'
2+
import format from 'pg-format'
23
import { coalesceRowsToArray, toTransaction } from '../lib/helpers'
34
import { RunQuery } from '../lib/connectionPool'
45
import { DEFAULT_SYSTEM_SCHEMAS } from '../lib/constants'
@@ -118,20 +119,25 @@ const getPolicyById = async (connection: string, id: number) => {
118119
let sql = `
119120
with policies as (${policies})
120121
select * from policies
121-
where policies.id = '${id}'
122+
where policies.id = ${id}
122123
limit 1
123124
`.trim()
124125
const { data } = await RunQuery(connection, sql)
125126
return data[0]
126127
}
127128
const getPolicyByName = async (connection: string, name: string, schema: string, table: string) => {
128129
const { policies } = sqlTemplates
129-
let sql = `
130-
with policies as (${policies})
130+
let sql = format(
131+
`
132+
with policies as (${policies})
131133
select * from policies
132-
where policies.name = '${name}' and policies.schema = '${schema}' and policies.table = '${table}'
134+
where policies.name = %L and policies.schema = %L and policies.table = %L
133135
limit 1
134-
`.trim()
136+
`,
137+
name,
138+
schema,
139+
table
140+
)
135141
const { data } = await RunQuery(connection, sql)
136142
return data[0]
137143
}
@@ -154,17 +160,22 @@ const createSql = ({
154160
command?: string
155161
roles?: string[]
156162
}) => {
157-
let sql = ` CREATE POLICY "${name}" ON "${schema}"."${table}"
163+
let sql = format(
164+
`CREATE POLICY %I ON %I.%I
158165
AS ${action}
159166
FOR ${command}
160-
TO ${roles.join(',')} `.trim()
167+
TO ${roles.join(',')} `,
168+
name,
169+
schema,
170+
table
171+
)
161172
if (definition) sql += ` USING (${definition}) `
162173
if (check) sql += ` WITH CHECK (${check}) `
163174
sql += ';'
164175
return sql
165176
}
166177
const alterPolicyNameSql = (oldName: string, newName: string, schema: string, table: string) => {
167-
return `ALTER POLICY "${oldName}" ON "${schema}"."${table}" RENAME TO "${newName}";`.trim()
178+
return format(`ALTER POLICY %I ON %I.%I RENAME TO %I;`, oldName, schema, table, newName)
168179
}
169180
const alterSql = ({
170181
name,
@@ -181,7 +192,7 @@ const alterSql = ({
181192
check?: string
182193
roles?: string[]
183194
}) => {
184-
let alter = `ALTER POLICY "${name}" ON "${schema}"."${table}"`
195+
let alter = format(`ALTER POLICY %I ON %I.%I`, name, schema, table)
185196
let newDefinition = definition !== undefined ? `${alter} USING (${definition});` : ''
186197
let newCheck = check !== undefined ? `${alter} WITH CHECK (${check});` : ''
187198
let newRoles = roles !== undefined ? `${alter} TO (${roles.join(',')});` : ''
@@ -192,7 +203,7 @@ const alterSql = ({
192203
${newRoles}`.trim()
193204
}
194205
const dropSql = (name: string, schema: string, table: string) => {
195-
return `DROP POLICY "${name}" ON "${schema}"."${table}";`.trim()
206+
return format(`DROP POLICY %I ON %I.%I;`, name, schema, table)
196207
}
197208
const removeSystemSchemas = (data: Tables.Policy[]) => {
198209
return data.filter((x) => !DEFAULT_SYSTEM_SCHEMAS.includes(x.schema))

‎src/api/roles.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Router } from 'express'
22

3+
import format from 'pg-format'
34
import SQL from 'sql-template-strings'
45
import sqlTemplates = require('../lib/sql')
56
const { grants, roles } = sqlTemplates
@@ -137,14 +138,15 @@ const createRoleSqlize = ({
137138
const isReplicationRoleSql = is_replication_role ? 'REPLICATION' : 'NOREPLICATION'
138139
const canBypassRlsSql = can_bypass_rls ? 'BYPASSRLS' : 'NOBYPASSRLS'
139140
const connectionLimitSql = `CONNECTION LIMIT ${connection_limit}`
140-
const passwordSql = password === undefined ? '' : `PASSWORD '${password}'`
141-
const validUntilSql = valid_until === undefined ? '' : `VALID UNTIL '${valid_until}'`
141+
const passwordSql = password === undefined ? '' : `PASSWORD ${format.literal(password)}`
142+
const validUntilSql =
143+
valid_until === undefined ? '' : `VALID UNTIL ${format.literal(valid_until)}`
142144
const memberOfSql = member_of === undefined ? '' : `IN ROLE ${member_of.join(',')}`
143145
const membersSql = members === undefined ? '' : `ROLE ${members.join(',')}`
144146
const adminsSql = admins === undefined ? '' : `ADMIN ${admins.join(',')}`
145147

146148
return `
147-
CREATE ROLE "${name}"
149+
CREATE ROLE ${format.ident(name)}
148150
WITH
149151
${isSuperuserSql}
150152
${canCreateDbSql}
@@ -193,7 +195,7 @@ const alterRoleSqlize = ({
193195
password?: string
194196
valid_until?: string
195197
}) => {
196-
const nameSql = name === undefined ? '' : `ALTER ROLE "${oldName}" RENAME TO "${name}";`
198+
const nameSql = name === undefined ? '' : format('ALTER ROLE %I RENAME TO %I;', oldName, name)
197199
let isSuperuserSql = ''
198200
if (is_superuser !== undefined) {
199201
isSuperuserSql = is_superuser ? 'SUPERUSER' : 'NOSUPERUSER'
@@ -224,12 +226,12 @@ const alterRoleSqlize = ({
224226
}
225227
const connectionLimitSql =
226228
connection_limit === undefined ? '' : `CONNECTION LIMIT ${connection_limit}`
227-
let passwordSql = password === undefined ? '' : `PASSWORD '${password}'`
228-
let validUntilSql = valid_until === undefined ? '' : `VALID UNTIL '${valid_until}'`
229+
let passwordSql = password === undefined ? '' : `PASSWORD ${format.literal(password)}`
230+
let validUntilSql = valid_until === undefined ? '' : `VALID UNTIL ${format.literal(valid_until)}`
229231

230232
return `
231233
BEGIN;
232-
ALTER ROLE "${oldName}"
234+
ALTER ROLE ${format.ident(oldName)}
233235
${isSuperuserSql}
234236
${canCreateDbSql}
235237
${canCreateRoleSql}
@@ -244,7 +246,7 @@ BEGIN;
244246
COMMIT;`
245247
}
246248
const dropRoleSqlize = (name: string) => {
247-
return `DROP ROLE "${name}"`
249+
return `DROP ROLE ${format.ident(name)}`
248250
}
249251
const removeSystemSchemas = (data: Roles.Role[]) => {
250252
return data.map((role) => {

‎src/api/schemas.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Router } from 'express'
2+
import format from 'pg-format'
23
import SQL from 'sql-template-strings'
34
import sqlTemplates = require('../lib/sql')
45
import { RunQuery } from '../lib/connectionPool'
@@ -110,7 +111,7 @@ const selectSingleByName = (name: string) => {
110111
return query
111112
}
112113
const createSchema = (name: string, owner: string = 'postgres') => {
113-
const query = SQL``.append(`CREATE SCHEMA IF NOT EXISTS "${name}" AUTHORIZATION ${owner}`)
114+
const query = SQL``.append(`CREATE SCHEMA IF NOT EXISTS ${name} AUTHORIZATION ${owner}`)
114115
return query
115116
}
116117
const alterSchemaName = (previousName: string, newName: string) => {
@@ -122,7 +123,7 @@ const alterSchemaOwner = (schemaName: string, newOwner: string) => {
122123
return query
123124
}
124125
const dropSchemaSqlize = (name: string, cascade: boolean) => {
125-
const query = `DROP SCHEMA "${name}" ${cascade ? 'CASCADE' : 'RESTRICT'}`
126+
const query = `DROP SCHEMA ${format.ident(name)} ${cascade ? 'CASCADE' : 'RESTRICT'}`
126127
return query
127128
}
128129
const removeSystemSchemas = (data: Schemas.Schema[]) => {

0 commit comments

Comments
 (0)