Improve SQL robustness and security across notification settings, private messages, and utilities

- Replaced raw SQL interpolation with parameterized queries to prevent injection risks.
- Refactored user notification preference validation using runtime synchronized enums for enhanced type safety.
- Updated private message and channel membership checks to respect the "left" status.
- Enhanced JSONB handling in update queries for correctness and maintainability.
This commit is contained in:
MartinBraquet
2026-07-23 14:14:26 +02:00
parent ba3af2e5d7
commit d69bc44dfd
9 changed files with 114 additions and 56 deletions

View File

@@ -393,7 +393,44 @@ function generateSwaggerPaths(api: typeof API) {
return paths
}
const apiKey = process.env.NEXT_PUBLIC_FIREBASE_API_KEY ?? 'API_KEY'
// const apiKey = process.env.NEXT_PUBLIC_FIREBASE_API_KEY ?? 'API_KEY'
// const accessInfo = `
// To obtain a token:
//
// **In your browser console while logged in (CTRL+SHIFT+C, then select the Console tab):**
// \`\`\`js
// const db = await new Promise((res, rej) => {
// const req = indexedDB.open('firebaseLocalStorageDb')
// req.onsuccess = () => res(req.result)
// req.onerror = rej
// })
// const req = db.transaction('firebaseLocalStorage', 'readonly').objectStore('firebaseLocalStorage').getAll()
// req.onsuccess = () => {
// const id_token = req.result[0].value.stsTokenManager.accessToken
// console.log('YOUR_FIREBASE_JWT_TOKEN is the string below:')
// console.log(id_token)
// copy(id_token)
// }
// \`\`\`
//
// **For testing (REST):**
// \`\`\`bash
// curl 'https://identitytoolkit.googleapis.com/v1/accounts:signInWithPassword?key=${apiKey}' \\
// -H 'Content-Type: application/json' \\
// --data '{"email":"you@example.com","password":"yourpassword","returnSecureToken":true}'
// # Use the returned idToken as your Bearer token
// \`\`\`
//
// Tokens expire after **1 hour**. Refresh by calling \`getIdToken(true)\`.
//
// Pass the token in the Authorization header for all authenticated requests:
// \`\`\`
// Authorization: Bearer YOUR_FIREBASE_JWT_TOKEN
// \`\`\`
//
// In the API docs, authenticate through the green button at the bottom right of this section (BearerAuth token).
// `
const swaggerDocument: OpenAPIV3.Document = {
openapi: '3.0.0',
@@ -410,41 +447,6 @@ Some endpoints are publicly accessible without authentication, such as events an
### Tier 2 — User Access (Firebase authentication required)
Most endpoints require a valid Firebase JWT token. This gives you access to your own user data, profile, messages, and all interactive features.
To obtain a token:
**In your browser console while logged in (CTRL+SHIFT+C, then select the Console tab):**
\`\`\`js
const db = await new Promise((res, rej) => {
const req = indexedDB.open('firebaseLocalStorageDb')
req.onsuccess = () => res(req.result)
req.onerror = rej
})
const req = db.transaction('firebaseLocalStorage', 'readonly').objectStore('firebaseLocalStorage').getAll()
req.onsuccess = () => {
const id_token = req.result[0].value.stsTokenManager.accessToken
console.log('YOUR_FIREBASE_JWT_TOKEN is the string below:')
console.log(id_token)
copy(id_token)
}
\`\`\`
**For testing (REST):**
\`\`\`bash
curl 'https://identitytoolkit.googleapis.com/v1/accounts:signInWithPassword?key=${apiKey}' \\
-H 'Content-Type: application/json' \\
--data '{"email":"you@example.com","password":"yourpassword","returnSecureToken":true}'
# Use the returned idToken as your Bearer token
\`\`\`
Tokens expire after **1 hour**. Refresh by calling \`getIdToken(true)\`.
Pass the token in the Authorization header for all authenticated requests:
\`\`\`
Authorization: Bearer YOUR_FIREBASE_JWT_TOKEN
\`\`\`
In the API docs, authenticate through the green button at the bottom right of this section (BearerAuth token).
**Don't have an account?** [Register on Compass](${DEPLOYED_WEB_URL}/register) to get started.
## Rate Limiting

View File

@@ -29,7 +29,8 @@ export async function getChannelMessages(props: {
and exists (select 1
from private_user_message_channel_members pumcm
where pumcm.user_id = $2
and pumcm.channel_id = $1)
and pumcm.channel_id = $1
and pumcm.status != 'left')
and ($4 is null or id > $4)
and ($5 is null or id < $5)
and not visibility = 'system_status'

View File

@@ -119,12 +119,14 @@ export const createPrivateUserMessageMain = async (
) => {
log('createPrivateUserMessageMain', creator, channelId, content)
// Normally, users can only submit messages to channels that they are members of
// Normally, users can only submit messages to channels that they are active members of
// (a member who has left keeps their row with status = 'left' and must not be able to post).
const authorized = await pg.oneOrNone(
`select 1
from private_user_message_channel_members
where channel_id = $1
and user_id = $2`,
and user_id = $2
and status != 'left'`,
[channelId, creator.id],
)
if (!authorized) throw APIErrors.forbidden('You are not authorized to post to this channel')

View File

@@ -15,7 +15,8 @@ export const reactToMessage: APIHandler<'react-to-message'> = async (
FROM private_user_message_channel_members m
JOIN private_user_messages msg ON msg.channel_id = m.channel_id
WHERE m.user_id = $1
AND msg.id = $2`,
AND msg.id = $2
AND m.status != 'left'`,
[auth.uid, messageId],
)

View File

@@ -14,12 +14,15 @@ export const updateNotifSettings: APIHandler<'update-notif-settings'> = async (
interestedInPushNotifications: !enabled,
})
} else {
// deep update array at data.notificationPreferences[type]
// deep update array at data.notificationPreferences[type].
// `type` and `medium` are passed as bound parameters (never interpolated into the SQL
// text) and are additionally constrained by the Zod enums on the endpoint schema. The
// jsonb path is built with array[...] so the key is a value, not raw SQL.
await pg.none(
`update private_users
set data = jsonb_set(data, '{notificationPreferences, $1:raw}',
set data = jsonb_set(data, array['notificationPreferences', $1],
coalesce(data->'notificationPreferences'->$1, '[]'::jsonb)
${enabled ? `|| '[$2:name]'::jsonb` : `- $2`}
${enabled ? `|| to_jsonb($2::text)` : `- $2`}
)
where id = $3`,
[type, medium, auth.uid],

View File

@@ -13,7 +13,7 @@ export const updatePrivateUserMessageChannel: APIHandler<
const membershipStatus = await pg.oneOrNone(
`select status from private_user_message_channel_members
where channel_id = $1 and user_id = $2`,
where channel_id = $1 and user_id = $2 and status != 'left'`,
[channelId, auth.uid],
)
if (!membershipStatus) throw APIErrors.forbidden('You are not authorized to this channel')

View File

@@ -88,7 +88,10 @@ export async function update<T extends TableName, ColumnValues extends Tables[T]
if (!(idField in values)) {
throw new Error(`missing ${idField} in values for ${columnNames}`)
}
const clause = pgp.as.format(`${idField} = $1`, values[idField as keyof ColumnValues])
const clause = pgp.as.format(`$(idField:name) = $(id)`, {
idField,
id: values[idField as keyof ColumnValues],
})
const query = pgp.helpers.update(values, cs) + ` WHERE ${clause}`
// Hack to properly cast values.
const q = query.replace(/::(\w*)'/g, "'::$1")
@@ -150,15 +153,22 @@ export async function bulkUpdateData<T extends TableName>(
updates: (Partial<DataFor<T>> & {id: string})[],
) {
if (updates.length > 0) {
// Each tuple is fully escaped by pg-promise ($(id) as a value, $(update:json) serialized
// and quoted) before being joined. The previous version interpolated `id` raw and used
// `.replace("'", "''")`, which only escapes the FIRST quote — both a correctness bug and
// an injection vector if any id/value contained a quote.
const values = updates
.map(({id, ...update}) => `('${id}', '${JSON.stringify(update).replace("'", "''")}'::jsonb)`)
.map(({id, ...update}) => pgp.as.format(`($(id), $(update:json)::jsonb)`, {id, update}))
.join(',\n')
await db.none(
`update ${table} as c
pgp.as.format(
`update $(table:name) as c
set data = data || v.update
from (values ${values}) as v(id, update)
from (values $(values:raw)) as v(id, update)
where c.id = v.id`,
{table, values},
),
)
}
}
@@ -185,12 +195,17 @@ export async function updateData<T extends TableName>(
}
const sortedExtraOperations = sortBy(extras, (statement) => (statement.startsWith('-') ? -1 : 1))
// `id` is bound as a value ($(id)) and `table`/`idField` as escaped identifiers (:name)
// rather than interpolated into the SQL text — otherwise an attacker-controlled id (e.g. a
// path param routed here via updatePrivateUser/updateUser) could break out of the
// `where ... = '...'` literal and run arbitrary SQL. The extra operations are already
// escaped by the FieldVal helpers that produced them.
return await db.one<Row<T>>(
`update ${table} set data = data
`update $(table:name) set data = data
${sortedExtraOperations.join('\n')}
|| $1
where ${idField} = '${id}' returning *`,
[JSON.stringify(basic)],
|| $(basic:json)
where $(idField:name) = $(id) returning *`,
{table, idField, id, basic},
)
}
@@ -216,9 +231,13 @@ export const FieldVal = {
arrayRemove:
(...values: string[]) =>
(fieldName: string) => {
// Build the text[] with an array[...] constructor whose elements are individually
// escaped by pg-promise (:list), rather than interpolating the joined values into the
// SQL with :raw, which does no escaping and let a value like `a'}'::text[]); drop ...`
// break out of the literal.
return pgp.as.format(
`|| jsonb_build_object($1, coalesce(data->$1,'[]'::jsonb) - '{$2:raw}'::text[])`,
[fieldName, values.join(',')],
`|| jsonb_build_object($1, coalesce(data->$1,'[]'::jsonb) - array[$2:list]::text[])`,
[fieldName, values],
)
},
}

View File

@@ -15,7 +15,7 @@ import {RepoStats, Stats} from 'common/stats' // mqp: very unscientific, just ba
import {PrivateMessageChannel} from 'common/supabase/private-messages'
import {Row} from 'common/supabase/utils'
import {PrivateUser, User} from 'common/user'
import {notification_preference} from 'common/user-notification-preferences'
import {NOTIFICATION_PREFERENCE_TYPES} from 'common/user-notification-preferences'
import {arrify} from 'common/util/array'
import {z} from 'zod'
@@ -423,7 +423,7 @@ export const API = (_apiTypeCheck = {
authed: true,
rateLimited: false,
props: z.object({
type: z.string() as z.ZodType<notification_preference>,
type: z.enum(NOTIFICATION_PREFERENCE_TYPES),
medium: z.enum(['email', 'browser', 'mobile']),
enabled: z.boolean(),
}),

View File

@@ -6,6 +6,36 @@ import {filterDefined} from './util/array'
export const NOTIFICATION_DESTINATION_TYPES = ['email', 'browser', 'mobile'] as const
export type notification_destination_types = (typeof NOTIFICATION_DESTINATION_TYPES)[number]
export type notification_preference = keyof notification_preferences
// Runtime allow-list of valid preference keys. Kept in sync with `notification_preferences`
// by the `satisfies` clause below — adding a key to the type without adding it here (or vice
// versa) is a compile error. Used to validate the untrusted `type` field on the wire so it
// never reaches a SQL statement unchecked.
export const NOTIFICATION_PREFERENCE_TYPES = [
'new_match',
'new_endorsement',
'new_profile_like',
'new_profile_ship',
'new_search_alerts',
'connection_interest_match',
'new_message',
'tagged_user',
'on_new_follow',
'onboarding_flow',
'thank_you_for_purchases',
'platform_updates',
'opt_out_all',
] as const satisfies readonly notification_preference[]
// Reverse check: every key of `notification_preferences` must appear in the list above.
// `satisfies` only guards the forward direction; this errors if a key is ever missing.
type _AllPreferenceTypesListed =
notification_preference extends (typeof NOTIFICATION_PREFERENCE_TYPES)[number]
? true
: ['missing notification_preference in NOTIFICATION_PREFERENCE_TYPES']
const _assertAllPreferenceTypesListed: _AllPreferenceTypesListed = true
void _assertAllPreferenceTypesListed
export type notification_preferences = {
new_match: notification_destination_types[]
new_endorsement: notification_destination_types[]