diff --git a/supabase/functions/_backend/public/bundle/set_channel.ts b/supabase/functions/_backend/public/bundle/set_channel.ts index c1e984ca86..c1a324c62d 100644 --- a/supabase/functions/_backend/public/bundle/set_channel.ts +++ b/supabase/functions/_backend/public/bundle/set_channel.ts @@ -2,6 +2,7 @@ import type { Context } from 'hono' import type { MiddlewareKeyVariables } from '../../utils/hono.ts' import type { Database } from '../../utils/supabase.types.ts' import { quickError, simpleError } from '../../utils/hono.ts' +import { closeClient, getPgClient, logPgError } from '../../utils/pg.ts' import { checkPermission } from '../../utils/rbac.ts' import { supabaseApikey } from '../../utils/supabase.ts' import { isValidAppId } from '../../utils/utils.ts' @@ -64,15 +65,58 @@ export async function setChannel(c: Context, body: SetCh throw simpleError('cannot_find_channel', 'Cannot find channel', { supabaseError: channelError }) } + const effectiveApikey = apikey.key ?? c.get('capgkey') + if (!effectiveApikey) { + throw simpleError('cannot_set_bundle_to_channel', 'Cannot set bundle to channel', { error: 'Missing API key context for audit logging' }) + } + // Update the channel to set the new version - const { error: updateError } = await supabaseApikey(c, apikey.key) - .from('channels') - .update({ version: body.version_id }) - .eq('id', body.channel_id) - .eq('app_id', body.app_id) + // Keep the supported write-scoped /bundle flow working after explicit RBAC + // and ownership checks while preserving API-key identity for audit triggers. + const pgClient = getPgClient(c) + let dbClient: { + query: (text: string, params?: unknown[]) => Promise<{ rowCount?: number | null }> + release: () => void + } | null = null + try { + dbClient = await pgClient.connect() + await dbClient.query('BEGIN') + await dbClient.query( + 'SELECT set_config(\'request.headers\', $1, true)', + [JSON.stringify({ capgkey: effectiveApikey })], + ) - if (updateError) { - throw simpleError('cannot_set_bundle_to_channel', 'Cannot set bundle to channel', { supabaseError: updateError }) + const updateResult = await dbClient.query( + `UPDATE public.channels + SET version = $1 + WHERE id = $2 + AND app_id = $3 + AND owner_org = $4 + RETURNING id`, + [body.version_id, body.channel_id, body.app_id, org.owner_org], + ) + + if ((updateResult.rowCount ?? 0) !== 1) { + throw new Error('Channel update affected 0 rows') + } + + await dbClient.query('COMMIT') + } + catch (error) { + if (dbClient) { + try { + await dbClient.query('ROLLBACK') + } + catch { + // Ignore rollback failures to preserve the original database error. + } + } + logPgError(c, 'set_channel_update', error) + throw simpleError('cannot_set_bundle_to_channel', 'Cannot set bundle to channel', { error: (error as Error)?.message }) + } + finally { + dbClient?.release() + await closeClient(c, pgClient) } return c.json({ diff --git a/supabase/migrations/20260424090727_block_apikey_channel_updates.sql b/supabase/migrations/20260424090727_block_apikey_channel_updates.sql new file mode 100644 index 0000000000..c561eb90e4 --- /dev/null +++ b/supabase/migrations/20260424090727_block_apikey_channel_updates.sql @@ -0,0 +1,27 @@ +-- Block direct PostgREST channel updates for write-scoped API keys. +-- Authenticated users keep their existing write access, and all-scoped API keys +-- still retain the direct channel update behavior expected by the CLI. + +DROP POLICY IF EXISTS "Allow update for auth, api keys (write, all) (write+)" ON public.channels; + +CREATE POLICY "Allow update for auth, api keys (write, all) (write+)" ON public.channels +FOR UPDATE +TO anon, authenticated +USING ( + public.check_min_rights( + 'write'::public.user_min_right, + public.get_identity_org_appid('{all}'::public.key_mode[], owner_org, app_id), + owner_org, + app_id, + NULL::bigint + ) +) +WITH CHECK ( + public.check_min_rights( + 'write'::public.user_min_right, + public.get_identity_org_appid('{all}'::public.key_mode[], owner_org, app_id), + owner_org, + app_id, + NULL::bigint + ) +); diff --git a/tests/audit-logs.test.ts b/tests/audit-logs.test.ts index 04b4d7fb03..18c84d1aa7 100644 --- a/tests/audit-logs.test.ts +++ b/tests/audit-logs.test.ts @@ -116,7 +116,7 @@ beforeAll(async () => { const { data: apiKeyData, error: apiKeyError } = await getSupabaseClient().rpc('create_hashed_apikey_for_user', { p_user_id: USER_ID, - p_mode: 'all', + p_mode: 'write', p_name: `audit-api-key-${globalId}`, p_limited_to_orgs: [ORG_ID], p_limited_to_apps: [APIKEY_AUDIT_APP_ID], @@ -583,3 +583,111 @@ describe('audit logs for app_versions via API key', () => { } }) }) + +describe('audit logs for channel promotions via API key bundle flow', () => { + const sourceVersionName = `99.0.0-audit-channel-source-${randomUUID()}` + const targetVersionName = `99.0.0-audit-channel-target-${randomUUID()}` + const channelName = `audit-channel-${randomUUID()}` + let sourceVersionId: number | null = null + let targetVersionId: number | null = null + let channelId: number | null = null + + beforeAll(async () => { + const supabase = getSupabaseClient() + + const { data: insertedVersions, error: versionError } = await supabase + .from('app_versions') + .insert([ + { + app_id: APIKEY_AUDIT_APP_ID, + name: sourceVersionName, + owner_org: ORG_ID, + }, + { + app_id: APIKEY_AUDIT_APP_ID, + name: targetVersionName, + owner_org: ORG_ID, + }, + ]) + .select('id,name') + + if (versionError || !insertedVersions || insertedVersions.length !== 2) { + throw new Error(`Failed to create audit channel versions: ${versionError?.message ?? 'missing versions'}`) + } + + sourceVersionId = insertedVersions.find(version => version.name === sourceVersionName)?.id ?? null + targetVersionId = insertedVersions.find(version => version.name === targetVersionName)?.id ?? null + if (sourceVersionId === null || targetVersionId === null) { + throw new Error('Failed to resolve audit channel version IDs') + } + + const { data: insertedChannel, error: channelError } = await supabase + .from('channels') + .insert({ + name: channelName, + app_id: APIKEY_AUDIT_APP_ID, + version: sourceVersionId, + created_by: USER_ID, + owner_org: ORG_ID, + }) + .select('id') + .single() + + if (channelError || !insertedChannel) { + throw new Error(`Failed to create audit channel: ${channelError?.message ?? 'missing channel'}`) + } + + channelId = insertedChannel.id + }) + + afterAll(async () => { + const supabase = getSupabaseClient() + + if (channelId !== null) { + await supabase.from('channels').delete().eq('id', channelId) + await supabase.from('audit_logs').delete().eq('record_id', channelId.toString()).eq('table_name', 'channels') + } + + if (sourceVersionId !== null) { + await supabase.from('app_versions').delete().eq('id', sourceVersionId) + } + + if (targetVersionId !== null) { + await supabase.from('app_versions').delete().eq('id', targetVersionId) + } + }) + + it('channel UPDATE via API key bundle promotion keeps audit user_id attribution', async () => { + if (!channelId || !targetVersionId) { + throw new Error('Audit channel promotion test setup did not complete') + } + const promotionChannelId = channelId + + const response = await fetchWithRetry(`${BASE_URL}/bundle`, { + method: 'PUT', + headers: apiKeyAuthHeaders, + body: JSON.stringify({ + app_id: APIKEY_AUDIT_APP_ID, + version_id: targetVersionId, + channel_id: promotionChannelId, + }), + }) + + expect(response.status).toBe(200) + + const promotionAuditLog = await waitForAuditLog( + `${BASE_URL}/organization/audit?orgId=${ORG_ID}&tableName=channels&operation=UPDATE`, + log => log.record_id === promotionChannelId.toString() && log.changed_fields?.includes('version') === true, + ) + + expect(promotionAuditLog.operation).toBe('UPDATE') + expect(promotionAuditLog.table_name).toBe('channels') + expect(promotionAuditLog.org_id).toBe(ORG_ID) + expect(promotionAuditLog.user_id).toBe(USER_ID) + expect(promotionAuditLog.changed_fields).toContain('version') + + if (promotionAuditLog.new_record && typeof promotionAuditLog.new_record === 'object') { + expect((promotionAuditLog.new_record as Record).version).toBe(targetVersionId) + } + }) +}) diff --git a/tests/bundle.test.ts b/tests/bundle.test.ts index 96cb35caae..d2fc14cb57 100644 --- a/tests/bundle.test.ts +++ b/tests/bundle.test.ts @@ -197,6 +197,8 @@ describe('[DELETE] /bundle operations', () => { describe('[PUT] /bundle operations - Set bundle to channel', () => { let versionId: number let channelId: number + let writeScopedKeyId: number | undefined + let writeScopedHeaders: Record | undefined beforeAll(async () => { // Create a test version @@ -235,6 +237,36 @@ describe('[PUT] /bundle operations - Set bundle to channel', () => { throw new Error('Failed to create test channel: channel is null') } channelId = channel.id + + const createKeyResponse = await fetch(`${BASE_URL}/apikey`, { + method: 'POST', + headers, + body: JSON.stringify({ + name: `bundle-write-key-${APPNAME}`, + mode: 'write', + limited_to_apps: [APPNAME], + }), + }) + + const createKeyData = await createKeyResponse.json() as { id: number, key: string } + if (createKeyResponse.status !== 200) { + throw new Error(`Failed to create write-scoped bundle key: ${JSON.stringify(createKeyData)}`) + } + + writeScopedKeyId = createKeyData.id + writeScopedHeaders = { + 'Content-Type': 'application/json', + 'capgkey': createKeyData.key, + } + }) + + afterAll(async () => { + if (writeScopedKeyId != null) { + await fetch(`${BASE_URL}/apikey/${writeScopedKeyId}`, { + method: 'DELETE', + headers, + }) + } }) it('should set bundle to channel successfully', async () => { @@ -266,6 +298,38 @@ describe('[PUT] /bundle operations - Set bundle to channel', () => { } }) + it('should keep the supported write-scoped API key bundle promotion flow working', async () => { + if (!writeScopedHeaders) { + throw new Error('Write-scoped bundle test key was not created') + } + + const response = await fetch(`${BASE_URL}/bundle`, { + method: 'PUT', + headers: writeScopedHeaders, + body: JSON.stringify({ + app_id: APPNAME, + version_id: versionId, + channel_id: channelId, + }), + }) + + const data = await response.json() as { status: string, message: string } + expect(response.status).toBe(200) + expect(data.status).toBe('success') + expect(data.message).toContain('set to channel') + + const supabase = getSupabaseClient() + const { data: channel, error } = await supabase + .from('channels') + .select('version') + .eq('id', channelId) + .eq('app_id', APPNAME) + .single() + + expect(error).toBeNull() + expect(channel?.version).toBe(versionId) + }) + it('should handle missing required fields', async () => { const response = await fetch(`${BASE_URL}/bundle`, { method: 'PUT', diff --git a/tests/hashed-apikey-rls.test.ts b/tests/hashed-apikey-rls.test.ts index bd5c72039e..e27cccd4cb 100644 --- a/tests/hashed-apikey-rls.test.ts +++ b/tests/hashed-apikey-rls.test.ts @@ -42,29 +42,37 @@ async function execWithCapgkey(sql: string, capgkey: string): Promise { } } -async function execWithAuthAndCapgkey( +type RequestRole = 'anon' | 'authenticated' + +async function execWithRoleClaims( sql: string, - userId: string, - capgkey: string, - params: unknown[] = [], + { + role, + claims, + headers, + params = [], + }: { + role: RequestRole + claims: Record + headers: Record + params?: unknown[] + }, ): Promise<{ rows: any[], rowCount: number }> { const client = await pool.connect() try { await client.query('BEGIN') try { - await client.query('SET LOCAL ROLE authenticated') - await client.query('SELECT set_config(\'request.jwt.claim.sub\', $1, true)', [userId]) + await client.query(`SET LOCAL ROLE ${role}`) + if (claims.sub) { + await client.query('SELECT set_config(\'request.jwt.claim.sub\', $1, true)', [claims.sub]) + } await client.query( 'SELECT set_config(\'request.jwt.claims\', $1, true)', - [JSON.stringify({ - sub: userId, - role: 'authenticated', - aud: 'authenticated', - })], + [JSON.stringify(claims)], ) await client.query( 'SELECT set_config(\'request.headers\', $1, true)', - [JSON.stringify({ capgkey })], + [JSON.stringify(headers)], ) const result = await client.query(sql, params) @@ -565,6 +573,119 @@ describe('rls policies with hashed api keys (via supabase sdk)', () => { }) }) +describe('channels rls blocks direct api-key updates', () => { + let allKey: { id: number, key: string, key_hash: string } | null = null + let writeKey: { id: number, key: string, key_hash: string } | null = null + let versionId: number | null = null + let channelId: number | null = null + const versionName = `rls-direct-version-${randomUUID().slice(0, 8)}` + const channelName = `rls-direct-channel-${randomUUID().slice(0, 8)}` + + beforeAll(async () => { + allKey = await createHashedApiKey('test-channel-direct-all-key', 'all', [ORG_ID_RLS], [APP_NAME_RLS]) + writeKey = await createHashedApiKey('test-channel-direct-write-key', 'write', [ORG_ID_RLS], [APP_NAME_RLS]) + + const versionResult = await pool.query( + `INSERT INTO public.app_versions (app_id, name, owner_org, user_id, checksum, storage_provider, r2_path, deleted) + VALUES ($1, $2, $3, $4, $5, $6, $7, false) + RETURNING id`, + [ + APP_NAME_RLS, + versionName, + ORG_ID_RLS, + USER_ID_RLS, + `checksum-${versionName}`, + 'r2', + `orgs/${ORG_ID_RLS}/apps/${APP_NAME_RLS}/${versionName}.zip`, + ], + ) + versionId = Number(versionResult.rows[0].id) + + const channelResult = await pool.query( + `INSERT INTO public.channels (app_id, name, version, owner_org, created_by, public, allow_emulator) + VALUES ($1, $2, $3, $4, $5, false, false) + RETURNING id`, + [ + APP_NAME_RLS, + channelName, + versionId, + ORG_ID_RLS, + USER_ID_RLS, + ], + ) + channelId = Number(channelResult.rows[0].id) + }, 60000) + + afterAll(async () => { + if (channelId) { + await pool.query('DELETE FROM public.channels WHERE id = $1', [channelId]) + } + + if (versionId) { + await pool.query('DELETE FROM public.app_versions WHERE id = $1', [versionId]) + } + + if (allKey) + await deleteApiKey(allKey.id) + + if (writeKey) + await deleteApiKey(writeKey.id) + }) + + it('does not let a write-scoped API key mutate protected channel fields via anon role access', async () => { + if (!writeKey || !channelId) + throw new Error('RLS channel test setup did not complete') + + const result = await execWithRoleClaims( + 'UPDATE public.channels SET allow_emulator = true WHERE id = $1 RETURNING id, allow_emulator', + { + role: 'anon', + claims: { + role: 'anon', + aud: 'anon', + }, + headers: { capgkey: writeKey.key }, + params: [channelId], + }, + ) + + expect(result.rowCount).toBe(0) + + const { rows } = await pool.query( + 'SELECT allow_emulator FROM public.channels WHERE id = $1', + [channelId], + ) + + expect(rows[0].allow_emulator).toBe(false) + }) + + it('still lets an all-scoped API key mutate supported channel fields via anon role access', async () => { + if (!allKey || !channelId) + throw new Error('RLS channel test setup did not complete') + + const result = await execWithRoleClaims( + 'UPDATE public.channels SET allow_emulator = true WHERE id = $1 RETURNING id, allow_emulator', + { + role: 'anon', + claims: { + role: 'anon', + aud: 'anon', + }, + headers: { capgkey: allKey.key }, + params: [channelId], + }, + ) + + expect(result.rowCount).toBe(1) + expect(result.rows[0].allow_emulator).toBe(true) + + await pool.query( + 'UPDATE public.channels SET allow_emulator = false WHERE id = $1', + [channelId], + ) + }) +}) + describe('webhook and webhook_delivery rls with api-key org scope precedence', () => { let limitedKey: { id: number, key: string, key_hash: string } let scopedKey: { id: number, key: string, key_hash: string } @@ -621,18 +742,32 @@ describe('webhook and webhook_delivery rls with api-key org scope precedence', ( }) it('uses API key org scope when auth context is also present for webhook reads', async () => { - const webhookRows = await execWithAuthAndCapgkey( + const webhookRows = await execWithRoleClaims( 'SELECT id FROM public.webhooks WHERE id = $1', - USER_ID_RLS, - limitedKey.key, - [webhookId], + { + role: 'authenticated', + claims: { + sub: USER_ID_RLS, + role: 'authenticated', + aud: 'authenticated', + }, + headers: { capgkey: limitedKey.key }, + params: [webhookId], + }, ).then(result => result.rows) - const deliveryRows = await execWithAuthAndCapgkey( + const deliveryRows = await execWithRoleClaims( 'SELECT id FROM public.webhook_deliveries WHERE id = $1', - USER_ID_RLS, - limitedKey.key, - [deliveryId], + { + role: 'authenticated', + claims: { + sub: USER_ID_RLS, + role: 'authenticated', + aud: 'authenticated', + }, + headers: { capgkey: limitedKey.key }, + params: [deliveryId], + }, ).then(result => result.rows) expect(webhookRows).toEqual([]) @@ -641,11 +776,18 @@ describe('webhook and webhook_delivery rls with api-key org scope precedence', ( it('prevents webhook_delivery org_id changes when update payload org_id is unauthorized', async () => { await expect( - execWithAuthAndCapgkey( + execWithRoleClaims( 'UPDATE public.webhook_deliveries SET org_id = $1 WHERE id = $2', - USER_ID_RLS, - scopedKey.key, - [ORG_ID_2, deliveryId], + { + role: 'authenticated', + claims: { + sub: USER_ID_RLS, + role: 'authenticated', + aud: 'authenticated', + }, + headers: { capgkey: scopedKey.key }, + params: [ORG_ID_2, deliveryId], + }, ), ).rejects.toMatchObject({ code: '42501' })