Skip to content

Commit 62d59d5

Browse files
charislamcharislam
authored
convert advisor and auth queries in pgmeta to safesql (supabase#44998)
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Improved SQL construction across the studio to make queries safer and more consistent. * Safer parameter handling for optional schema and remediation links to prevent injection risks. * Deterministic query header formatting and stable date/comments in generated SQL. * More robust user-count and paginated-user queries for accurate counts, sorting and pagination. * Updated tests to align with the new safe query handling. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 8ddd809 commit 62d59d5

4 files changed

Lines changed: 142 additions & 113 deletions

File tree

‎packages/pg-meta/src/sql/studio/advisor/lints.ts‎

Lines changed: 33 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,18 @@
1-
export const enrichLintsQuery = (query: string, exposedSchemas?: string) => {
2-
return `
1+
import { literal, safeSql, type SafeSqlFragment } from '../../../pg-format'
2+
3+
export const enrichLintsQuery = (query: SafeSqlFragment, exposedSchemas?: string) => {
4+
return safeSql`
35
do $$
46
begin
57
set pg_stat_statements.track = none;
68
exception when others then
79
-- not a superuser or extension not installed, skip silently
810
end
911
$$;
10-
${!!exposedSchemas ? `set local pgrst.db_schemas = '${exposedSchemas}';` : ''}
12+
${!!exposedSchemas ? safeSql`set local pgrst.db_schemas = ${literal(exposedSchemas)};` : safeSql``}
1113
-- source: dashboard
12-
-- user: ${'self host'}
13-
-- date: ${new Date().toISOString()}
14+
-- user: self host
15+
-- date: ${new Date().toISOString() as SafeSqlFragment}
1416
1517
${query}
1618
`
@@ -22,8 +24,8 @@ ${query}
2224
* - Replace all "\`%s\`" with backquotes to escape the tick character ("\`%s\`")
2325
* - Replace docs url with docsUrl (${docsUrl})
2426
*/
25-
export const getLintsSQL = ({ docsUrl }: { docsUrl: string }) =>
26-
/* SQL */ `set local search_path = '';
27+
export const getLintsSQL = ({ docsUrl }: { docsUrl: string }): SafeSqlFragment =>
28+
safeSql`set local search_path = '';
2729
2830
(
2931
with foreign_keys as (
@@ -70,7 +72,7 @@ select
7072
fk.table_name,
7173
fk.fkey_name
7274
) as detail,
73-
'${docsUrl}/guides/database/database-linter?lint=0001_unindexed_foreign_keys' as remediation,
75+
${literal(`${docsUrl}/guides/database/database-linter?lint=0001_unindexed_foreign_keys`)} as remediation,
7476
jsonb_build_object(
7577
'schema', fk.schema_name,
7678
'name', fk.table_name,
@@ -110,7 +112,7 @@ select
110112
'View/Materialized View "%s" in the public schema may expose \`auth.users\` data to anon or authenticated roles.',
111113
c.relname
112114
) as detail,
113-
'${docsUrl}/guides/database/database-linter?lint=0002_auth_users_exposed' as remediation,
115+
${literal(`${docsUrl}/guides/database/database-linter?lint=0002_auth_users_exposed`)} as remediation,
114116
jsonb_build_object(
115117
'schema', n.nspname,
116118
'name', c.relname,
@@ -224,12 +226,12 @@ select
224226
array['PERFORMANCE'] as categories,
225227
'Detects if calls to \`current_setting()\` and \`auth.<function>()\` in RLS policies are being unnecessarily re-evaluated for each row' as description,
226228
format(
227-
'Table \`%s.%s\` has a row level security policy \`%s\` that re-evaluates current_setting() or auth.<function>() for each row. This produces suboptimal query performance at scale. Resolve the issue by replacing \`auth.<function>()\` with \`(select auth.<function>())\`. See [docs](${docsUrl}/guides/database/postgres/row-level-security#call-functions-with-select) for more info.',
229+
${literal(`Table \`%s.%s\` has a row level security policy \`%s\` that re-evaluates current_setting() or auth.<function>() for each row. This produces suboptimal query performance at scale. Resolve the issue by replacing \`auth.<function>()\` with \`(select auth.<function>())\`. See [docs](${docsUrl}/guides/database/postgres/row-level-security#call-functions-with-select) for more info.`)},
228230
schema_name,
229231
table_name,
230232
policy_name
231233
) as detail,
232-
'${docsUrl}/guides/database/database-linter?lint=0003_auth_rls_initplan' as remediation,
234+
${literal(`${docsUrl}/guides/database/database-linter?lint=0003_auth_rls_initplan`)} as remediation,
233235
jsonb_build_object(
234236
'schema', schema_name,
235237
'name', table_name,
@@ -301,7 +303,7 @@ select
301303
pgns.nspname,
302304
pgc.relname
303305
) as detail,
304-
'${docsUrl}/guides/database/database-linter?lint=0004_no_primary_key' as remediation,
306+
${literal(`${docsUrl}/guides/database/database-linter?lint=0004_no_primary_key`)} as remediation,
305307
jsonb_build_object(
306308
'schema', pgns.nspname,
307309
'name', pgc.relname,
@@ -348,7 +350,7 @@ select
348350
psui.schemaname,
349351
psui.relname
350352
) as detail,
351-
'${docsUrl}/guides/database/database-linter?lint=0005_unused_index' as remediation,
353+
${literal(`${docsUrl}/guides/database/database-linter?lint=0005_unused_index`)} as remediation,
352354
jsonb_build_object(
353355
'schema', psui.schemaname,
354356
'name', psui.relname,
@@ -393,7 +395,7 @@ select
393395
act.cmd,
394396
array_agg(p.polname order by p.polname)
395397
) as detail,
396-
'${docsUrl}/guides/database/database-linter?lint=0006_multiple_permissive_policies' as remediation,
398+
${literal(`${docsUrl}/guides/database/database-linter?lint=0006_multiple_permissive_policies`)} as remediation,
397399
jsonb_build_object(
398400
'schema', n.nspname,
399401
'name', c.relname,
@@ -464,7 +466,7 @@ select
464466
c.relname,
465467
array_agg(p.polname order by p.polname)
466468
) as detail,
467-
'${docsUrl}/guides/database/database-linter?lint=0007_policy_exists_rls_disabled' as remediation,
469+
${literal(`${docsUrl}/guides/database/database-linter?lint=0007_policy_exists_rls_disabled`)} as remediation,
468470
jsonb_build_object(
469471
'schema', n.nspname,
470472
'name', c.relname,
@@ -509,7 +511,7 @@ select
509511
n.nspname,
510512
c.relname
511513
) as detail,
512-
'${docsUrl}/guides/database/database-linter?lint=0008_rls_enabled_no_policy' as remediation,
514+
${literal(`${docsUrl}/guides/database/database-linter?lint=0008_rls_enabled_no_policy`)} as remediation,
513515
jsonb_build_object(
514516
'schema', n.nspname,
515517
'name', c.relname,
@@ -556,7 +558,7 @@ select
556558
c.relname,
557559
array_agg(pi.indexname order by pi.indexname)
558560
) as detail,
559-
'${docsUrl}/guides/database/database-linter?lint=0009_duplicate_index' as remediation,
561+
${literal(`${docsUrl}/guides/database/database-linter?lint=0009_duplicate_index`)} as remediation,
560562
jsonb_build_object(
561563
'schema', n.nspname,
562564
'name', c.relname,
@@ -610,7 +612,7 @@ select
610612
n.nspname,
611613
c.relname
612614
) as detail,
613-
'${docsUrl}/guides/database/database-linter?lint=0010_security_definer_view' as remediation,
615+
${literal(`${docsUrl}/guides/database/database-linter?lint=0010_security_definer_view`)} as remediation,
614616
jsonb_build_object(
615617
'schema', n.nspname,
616618
'name', c.relname,
@@ -663,7 +665,7 @@ select
663665
n.nspname,
664666
p.proname
665667
) as detail,
666-
'${docsUrl}/guides/database/database-linter?lint=0011_function_search_path_mutable' as remediation,
668+
${literal(`${docsUrl}/guides/database/database-linter?lint=0011_function_search_path_mutable`)} as remediation,
667669
jsonb_build_object(
668670
'schema', n.nspname,
669671
'name', p.proname,
@@ -707,7 +709,7 @@ select
707709
n.nspname,
708710
c.relname
709711
) as detail,
710-
'${docsUrl}/guides/database/database-linter?lint=0013_rls_disabled_in_public' as remediation,
712+
${literal(`${docsUrl}/guides/database/database-linter?lint=0013_rls_disabled_in_public`)} as remediation,
711713
jsonb_build_object(
712714
'schema', n.nspname,
713715
'name', c.relname,
@@ -747,7 +749,7 @@ select
747749
'Extension \`%s\` is installed in the public schema. Move it to another schema.',
748750
pe.extname
749751
) as detail,
750-
'${docsUrl}/guides/database/database-linter?lint=0014_extension_in_public' as remediation,
752+
${literal(`${docsUrl}/guides/database/database-linter?lint=0014_extension_in_public`)} as remediation,
751753
jsonb_build_object(
752754
'schema', pe.extnamespace::regnamespace,
753755
'name', pe.extname,
@@ -801,7 +803,7 @@ select
801803
table_name,
802804
policy_name
803805
) as detail,
804-
'${docsUrl}/guides/database/database-linter?lint=0015_rls_references_user_metadata' as remediation,
806+
${literal(`${docsUrl}/guides/database/database-linter?lint=0015_rls_references_user_metadata`)} as remediation,
805807
jsonb_build_object(
806808
'schema', schema_name,
807809
'name', table_name,
@@ -837,7 +839,7 @@ select
837839
n.nspname,
838840
c.relname
839841
) as detail,
840-
'${docsUrl}/guides/database/database-linter?lint=0016_materialized_view_in_api' as remediation,
842+
${literal(`${docsUrl}/guides/database/database-linter?lint=0016_materialized_view_in_api`)} as remediation,
841843
jsonb_build_object(
842844
'schema', n.nspname,
843845
'name', c.relname,
@@ -880,7 +882,7 @@ select
880882
n.nspname,
881883
c.relname
882884
) as detail,
883-
'${docsUrl}/guides/database/database-linter?lint=0017_foreign_table_in_api' as remediation,
885+
${literal(`${docsUrl}/guides/database/database-linter?lint=0017_foreign_table_in_api`)} as remediation,
884886
jsonb_build_object(
885887
'schema', n.nspname,
886888
'name', c.relname,
@@ -925,7 +927,7 @@ select
925927
a.attname,
926928
t.typname
927929
) as detail,
928-
'${docsUrl}/guides/database/database-linter?lint=unsupported_reg_types' as remediation,
930+
${literal(`${docsUrl}/guides/database/database-linter?lint=unsupported_reg_types`)} as remediation,
929931
jsonb_build_object(
930932
'schema', n.nspname,
931933
'name', c.relname,
@@ -966,7 +968,7 @@ select
966968
n.nspname,
967969
c.relname
968970
) as detail,
969-
'${docsUrl}/guides/database/database-linter?lint=0019_insecure_queue_exposed_in_api' as remediation,
971+
${literal(`${docsUrl}/guides/database/database-linter?lint=0019_insecure_queue_exposed_in_api`)} as remediation,
970972
jsonb_build_object(
971973
'schema', n.nspname,
972974
'name', c.relname,
@@ -1149,7 +1151,7 @@ select
11491151
ext.installed_version,
11501152
ext.default_version
11511153
) as detail,
1152-
'${docsUrl}/guides/database/database-linter?lint=0022_extension_versions_outdated' as remediation,
1154+
${literal(`${docsUrl}/guides/database/database-linter?lint=0022_extension_versions_outdated`)} as remediation,
11531155
jsonb_build_object(
11541156
'extension_name', ext.name,
11551157
'installed_version', ext.installed_version,
@@ -1260,7 +1262,7 @@ select
12601262
table_name,
12611263
string_agg(distinct column_name, ', ' order by column_name)
12621264
) as detail,
1263-
'${docsUrl}/guides/database/database-linter?lint=0023_sensitive_columns_exposed' as remediation,
1265+
${literal(`${docsUrl}/guides/database/database-linter?lint=0023_sensitive_columns_exposed`)} as remediation,
12641266
jsonb_build_object(
12651267
'schema', schema_name,
12661268
'name', table_name,
@@ -1382,7 +1384,7 @@ select
13821384
end,
13831385
array_to_string(roles, ', ')
13841386
) as detail,
1385-
'${docsUrl}/guides/database/database-linter?lint=0024_permissive_rls_policy' as remediation,
1387+
${literal(`${docsUrl}/guides/database/database-linter?lint=0024_permissive_rls_policy`)} as remediation,
13861388
jsonb_build_object(
13871389
'schema', schema_name,
13881390
'name', table_name,
@@ -1491,7 +1493,7 @@ select
14911493
end,
14921494
array_to_string(policy_names, ', ')
14931495
) as detail,
1494-
'${docsUrl}/guides/database/database-linter?lint=0025_public_bucket_allows_listing' as remediation,
1496+
${literal(`${docsUrl}/guides/database/database-linter?lint=0025_public_bucket_allows_listing`)} as remediation,
14951497
jsonb_build_object(
14961498
'schema', 'storage',
14971499
'name', bucket_name,
@@ -1505,4 +1507,4 @@ select
15051507
from
15061508
affected_buckets
15071509
order by
1508-
bucket_id)`.trim()
1510+
bucket_id)`

‎packages/pg-meta/src/sql/studio/auth/get-users-count.ts‎

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
import { literal } from '../../../pg-format'
1+
import { joinSqlFragments, literal, safeSql, type SafeSqlFragment } from '../../../pg-format'
22
import { COUNT_ESTIMATE_SQL, THRESHOLD_COUNT } from '../database/get-count-estimate'
33
import { prefixToUUID, stringRange } from './get-users-common'
44
import type { OptimizedSearchColumns } from './get-users-types'
55

6-
export const USERS_COUNT_ESTIMATE_SQL = `select reltuples as estimate from pg_class where oid = 'auth.users'::regclass`
6+
export const USERS_COUNT_ESTIMATE_SQL = safeSql`select reltuples as estimate from pg_class where oid = 'auth.users'::regclass`
77

88
export const getUsersCountSQL = ({
99
filter,
@@ -18,12 +18,12 @@ export const getUsersCountSQL = ({
1818
forceExactCount?: boolean
1919
/** If set, uses optimized prefix search for the specified column */
2020
column?: OptimizedSearchColumns
21-
}) => {
21+
}): SafeSqlFragment => {
2222
const hasValidKeywords = keywords && keywords !== ''
2323

24-
const conditions: string[] = []
25-
const baseQueryCount = `select count(*) from auth.users`
26-
const baseQuerySelect = `select * from auth.users`
24+
const conditions: SafeSqlFragment[] = []
25+
const baseQueryCount = safeSql`select count(*) from auth.users`
26+
const baseQuerySelect = safeSql`select * from auth.users`
2727

2828
const optimizedSearchMode = column && hasValidKeywords
2929
if (optimizedSearchMode) {
@@ -32,38 +32,40 @@ export const getUsersCountSQL = ({
3232
const lowerBound = literal(range[0])
3333
const upperBound = range[1] ? literal(range[1]) : null
3434
conditions.push(
35-
`lower(email) >= ${lowerBound}${upperBound ? ` and lower(email) < ${upperBound}` : ''} and instance_id = '00000000-0000-0000-0000-000000000000'::uuid`
35+
safeSql`lower(email) >= ${lowerBound}${upperBound ? safeSql` and lower(email) < ${upperBound}` : safeSql``} and instance_id = '00000000-0000-0000-0000-000000000000'::uuid`
3636
)
3737
} else if (column === 'phone') {
3838
const range = stringRange(keywords)
3939
const lowerBound = literal(range[0])
4040
const upperBound = range[1] ? literal(range[1]) : null
41-
conditions.push(`phone >= ${lowerBound}${upperBound ? ` and phone < ${upperBound}` : ''}`)
41+
conditions.push(
42+
safeSql`phone >= ${lowerBound}${upperBound ? safeSql` and phone < ${upperBound}` : safeSql``}`
43+
)
4244
} else if (column === 'id') {
4345
const lowerUUID = prefixToUUID(keywords, false)
4446
const isMatchingUUIDValue = lowerUUID === keywords
4547
if (isMatchingUUIDValue) {
46-
conditions.push(`id = ${literal(keywords)}`)
48+
conditions.push(safeSql`id = ${literal(keywords)}`)
4749
} else {
4850
const upperUUID = prefixToUUID(keywords, true)
49-
conditions.push(`id >= ${literal(lowerUUID)} and id < ${literal(upperUUID)}`)
51+
conditions.push(safeSql`id >= ${literal(lowerUUID)} and id < ${literal(upperUUID)}`)
5052
}
5153
}
5254
} else {
5355
// Unified search mode - apply all filters
5456
if (hasValidKeywords) {
5557
const escapedFilterKeywords = literal(`%${keywords}%`)
5658
conditions.push(
57-
`id::text ilike ${escapedFilterKeywords} or email ilike ${escapedFilterKeywords} or phone ilike ${escapedFilterKeywords}`
59+
safeSql`id::text ilike ${escapedFilterKeywords} or email ilike ${escapedFilterKeywords} or phone ilike ${escapedFilterKeywords}`
5860
)
5961
}
6062

6163
if (filter === 'verified') {
62-
conditions.push(`email_confirmed_at IS NOT NULL or phone_confirmed_at IS NOT NULL`)
64+
conditions.push(safeSql`email_confirmed_at IS NOT NULL or phone_confirmed_at IS NOT NULL`)
6365
} else if (filter === 'anonymous') {
64-
conditions.push(`is_anonymous is true`)
66+
conditions.push(safeSql`is_anonymous is true`)
6567
} else if (filter === 'unverified') {
66-
conditions.push(`email_confirmed_at IS NULL AND phone_confirmed_at IS NULL`)
68+
conditions.push(safeSql`email_confirmed_at IS NULL AND phone_confirmed_at IS NULL`)
6769
}
6870

6971
if (providers && providers.length > 0) {
@@ -72,38 +74,41 @@ export const getUsersCountSQL = ({
7274
if (providers.includes('saml 2.0')) {
7375
const mappedProviders = providers.map((p) => (p === 'saml 2.0' ? 'sso' : p))
7476
conditions.push(
75-
`(select jsonb_agg(case when value ~ '^sso' then 'sso' else value end) from jsonb_array_elements_text((raw_app_meta_data ->> 'providers')::jsonb)) ?| array[${literal(mappedProviders)}]`.trim()
77+
safeSql`(select jsonb_agg(case when value ~ '^sso' then 'sso' else value end) from jsonb_array_elements_text((raw_app_meta_data ->> 'providers')::jsonb)) ?| array[${literal(mappedProviders)}]`
7678
)
7779
} else {
78-
conditions.push(`(raw_app_meta_data->>'providers')::jsonb ?| array[${literal(providers)}]`)
80+
conditions.push(
81+
safeSql`(raw_app_meta_data->>'providers')::jsonb ?| array[${literal(providers)}]`
82+
)
7983
}
8084
}
8185
}
8286

83-
const combinedConditions = conditions.map((x) => `(${x})`).join(' and ')
84-
const whereClause = conditions.length > 0 ? ` where ${combinedConditions}` : ''
87+
const combinedConditions = joinSqlFragments(
88+
conditions.map((x) => safeSql`(${x})`),
89+
' and '
90+
)
91+
const whereClause = conditions.length > 0 ? safeSql` where ${combinedConditions}` : safeSql``
8592

8693
if (forceExactCount) {
87-
return `select (${baseQueryCount}${whereClause}), false as is_estimate;`
94+
return safeSql`select (${baseQueryCount}${whereClause}), false as is_estimate;`
8895
} else {
89-
const selectBaseSql = `${baseQuerySelect}${whereClause}`
90-
const countBaseSql = `${baseQueryCount}${whereClause}`
96+
const selectBaseSql = safeSql`${baseQuerySelect}${whereClause}`
97+
const countBaseSql = safeSql`${baseQueryCount}${whereClause}`
9198

9299
const escapedSelectSql = literal(selectBaseSql)
93100

94-
const sql = `
95-
${COUNT_ESTIMATE_SQL}
101+
const sql = safeSql`${COUNT_ESTIMATE_SQL}
96102
97103
with approximation as (${USERS_COUNT_ESTIMATE_SQL})
98104
select
99105
case
100106
when estimate = -1 then (select pg_temp.count_estimate(${escapedSelectSql}))::int
101-
when estimate > ${THRESHOLD_COUNT} then ${conditions.length > 0 ? `(select pg_temp.count_estimate(${escapedSelectSql}))::int` : 'estimate::int'}
107+
when estimate > ${literal(THRESHOLD_COUNT)} then ${conditions.length > 0 ? safeSql`(select pg_temp.count_estimate(${escapedSelectSql}))::int` : safeSql`estimate::int`}
102108
else (${countBaseSql})
103109
end as count,
104-
estimate = -1 or estimate > ${THRESHOLD_COUNT} as is_estimate
105-
from approximation;
106-
`.trim()
110+
estimate = -1 or estimate > ${literal(THRESHOLD_COUNT)} as is_estimate
111+
from approximation`
107112

108113
return sql
109114
}

0 commit comments

Comments
 (0)