From a3968361c1ec0f83131699cfe017d2a1db1d481a Mon Sep 17 00:00:00 2001 From: Martin Donadieu Date: Sat, 2 May 2026 15:59:15 +0200 Subject: [PATCH] fix(webhooks): block app-scoped org actions --- .../_backend/public/webhooks/index.ts | 65 ++++++++------- supabase/functions/_backend/utils/supabase.ts | 2 + tests/webhooks.test.ts | 82 +++++++++++++++++++ 3 files changed, 119 insertions(+), 30 deletions(-) diff --git a/supabase/functions/_backend/public/webhooks/index.ts b/supabase/functions/_backend/public/webhooks/index.ts index b0af46f03b..0b9ef46c8f 100644 --- a/supabase/functions/_backend/public/webhooks/index.ts +++ b/supabase/functions/_backend/public/webhooks/index.ts @@ -44,6 +44,39 @@ async function assertWebhookOrgPolicy( throw simpleError('invalid_org_id', 'You can\'t access this organization', { org_id: orgId }) } +function uniqueApiKeys( + apikeys: (Database['public']['Tables']['apikeys']['Row'] | null | undefined)[], +) { + const filteredApiKeys = apikeys.filter((apikey): apikey is Database['public']['Tables']['apikeys']['Row'] => !!apikey) + return filteredApiKeys.filter((apikey, index) => filteredApiKeys.findIndex(existing => existing.id === apikey.id) === index) +} + +function getWebhookApiKeyChain(c: Context, apikey: Database['public']['Tables']['apikeys']['Row']) { + const parentApikey = c.get('parentApikey') as Database['public']['Tables']['apikeys']['Row'] | undefined + return uniqueApiKeys([parentApikey, apikey]) +} + +function getWebhookAuthApiKeyChain(c: Context, auth: AuthInfo) { + if (auth.authType !== 'apikey' || !auth.apikey) + return [] + + return getWebhookApiKeyChain(c, auth.apikey) +} + +async function assertWebhookApiKeyChain( + c: Context, + orgId: string, + apiKeyChain: Database['public']['Tables']['apikeys']['Row'][], +) { + for (const apikey of apiKeyChain) { + assertOrgWebhookScope(orgId, apikey) + } + + for (const apikey of apiKeyChain) { + await assertWebhookOrgPolicy(c, orgId, apikey) + } +} + /** * Shared permission check for webhook endpoints (API key auth) * Validates admin access to organization @@ -53,21 +86,11 @@ export async function checkWebhookPermission( orgId: string, apikey: Database['public']['Tables']['apikeys']['Row'], ): Promise { - const orgCheck = await apikeyHasOrgRightWithPolicy(c, apikey, orgId, supabaseApikey(c, c.get('capgkey') as string)) - if (!orgCheck.valid) { - if (orgCheck.error === 'org_requires_expiring_key') { - throw quickError(401, 'org_requires_expiring_key', 'This organization requires API keys with an expiration date. Please use a different key or update this key with an expiration date.') - } - throw simpleError('invalid_org_id', 'You can\'t access this organization', { org_id: orgId }) - } + await assertWebhookApiKeyChain(c, orgId, getWebhookApiKeyChain(c, apikey)) if (!(await hasOrgRightApikey(c, orgId, apikey.user_id, 'admin', c.get('capgkey') as string))) { throw simpleError('no_permission', 'You need admin access to manage webhooks', { org_id: orgId }) } - - if (apikey.limited_to_apps?.length) { - throw simpleError('no_permission', 'App-scoped API keys cannot manage organization webhooks', { org_id: orgId }) - } } /** @@ -79,18 +102,7 @@ export async function checkWebhookPermissionV2( orgId: string, auth: AuthInfo, ): Promise { - const parentApikey = c.get('parentApikey') as Database['public']['Tables']['apikeys']['Row'] | undefined - const policyApikey = parentApikey ?? auth.apikey - - if (auth.authType === 'apikey' && policyApikey) { - const orgCheck = await apikeyHasOrgRightWithPolicy(c, policyApikey, orgId, supabaseApikey(c, c.get('capgkey') as string)) - if (!orgCheck.valid) { - if (orgCheck.error === 'org_requires_expiring_key') { - throw quickError(401, 'org_requires_expiring_key', 'This organization requires API keys with an expiration date. Please use a different key or update this key with an expiration date.') - } - throw simpleError('invalid_org_id', 'You can\'t access this organization', { org_id: orgId }) - } - } + await assertWebhookApiKeyChain(c, orgId, getWebhookAuthApiKeyChain(c, auth)) const hasWebhookAdminRight = auth.authType === 'apikey' ? await hasOrgRightApikey(c, orgId, auth.userId, 'admin', c.get('capgkey') as string) @@ -99,13 +111,6 @@ export async function checkWebhookPermissionV2( if (!hasWebhookAdminRight) { throw simpleError('no_permission', 'You need admin access to manage webhooks', { org_id: orgId }) } - - // If using API key, also check the key has org access - if (auth.authType === 'apikey' && auth.apikey) { - assertOrgWebhookScope(orgId, auth.apikey) - const policyKey = c.get('apikey') as Database['public']['Tables']['apikeys']['Row'] | undefined - await assertWebhookOrgPolicy(c, orgId, policyKey ?? auth.apikey) - } } // List all webhooks for org diff --git a/supabase/functions/_backend/utils/supabase.ts b/supabase/functions/_backend/utils/supabase.ts index c229f4a1a7..1cb593b39b 100644 --- a/supabase/functions/_backend/utils/supabase.ts +++ b/supabase/functions/_backend/utils/supabase.ts @@ -296,6 +296,8 @@ export async function hasAppRightApikey(c: Context { // Create stripe_info for this test org @@ -63,6 +64,20 @@ beforeAll(async () => { appScopedKeyId = appScopedKeyData.id appScopedKey = appScopedKeyData.key + + const { data: orgScopedSubkeyData, error: orgScopedSubkeyError } = await getSupabaseClient().rpc('create_hashed_apikey_for_user', { + p_user_id: USER_ID, + p_mode: 'all', + p_name: `webhook-org-scoped-subkey-${globalId}`, + p_limited_to_orgs: [WEBHOOK_TEST_ORG_ID], + p_limited_to_apps: [], + p_expires_at: null as unknown as string, + }) + if (orgScopedSubkeyError || !orgScopedSubkeyData?.id) { + throw new Error(`Failed to create org-scoped API subkey for webhook tests: ${orgScopedSubkeyError?.message ?? 'missing key data'}`) + } + + orgScopedSubkeyId = orgScopedSubkeyData.id }) afterAll(async () => { @@ -74,6 +89,9 @@ afterAll(async () => { if (appScopedKeyId) { await getSupabaseClient().from('apikeys').delete().eq('id', appScopedKeyId) } + if (orgScopedSubkeyId) { + await getSupabaseClient().from('apikeys').delete().eq('id', orgScopedSubkeyId) + } await getSupabaseClient().from('apps').delete().eq('app_id', webhookAppId) // Clean up test organization and stripe_info await getSupabaseClient().from('orgs').delete().eq('id', WEBHOOK_TEST_ORG_ID) @@ -106,6 +124,24 @@ describe('[GET] /webhooks', () => { }) expect(response.status).toBe(400) }) + + it('rejects app-scoped parent keys with org-scoped subkeys for webhook listing', async () => { + if (!appScopedKey || !orgScopedSubkeyId) + throw new Error('Webhook subkey list prerequisites were not created') + + const response = await fetchWithRetry(`${BASE_URL}/webhooks?orgId=${WEBHOOK_TEST_ORG_ID}`, { + headers: { + 'Content-Type': 'application/json', + 'authorization': appScopedKey, + 'x-limited-key-id': String(orgScopedSubkeyId), + }, + }) + + expect(response.status).toBe(400) + const data = await response.json() as { error: string, message: string } + expect(data.error).toBe('no_permission') + expect(data.message).toContain('App-scoped API keys') + }) }) describe('[POST] /webhooks', () => { @@ -496,6 +532,29 @@ describe('[POST] /webhooks/test', () => { expect(data.error).toBe('no_permission') expect(data.message).toContain('App-scoped API keys') }) + + it('rejects app-scoped parent keys with org-scoped subkeys for webhook tests', async () => { + if (!createdWebhookId || !appScopedKey || !orgScopedSubkeyId) + throw new Error('Webhook subkey test prerequisites were not created') + + const response = await fetch(`${BASE_URL}/webhooks/test`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'x-api-key': appScopedKey, + 'x-limited-key-id': String(orgScopedSubkeyId), + }, + body: JSON.stringify({ + orgId: WEBHOOK_TEST_ORG_ID, + webhookId: createdWebhookId, + }), + }) + + expect(response.status).toBe(400) + const data = await response.json() as { error: string, message: string } + expect(data.error).toBe('no_permission') + expect(data.message).toContain('App-scoped API keys') + }) }) describe('[GET] /webhooks/deliveries', () => { @@ -596,6 +655,29 @@ describe('[POST] /webhooks/deliveries/retry', () => { expect(data.message).toContain('App-scoped API keys') }) + it('rejects app-scoped parent keys with org-scoped subkeys for delivery retries', async () => { + if (!lastDeliveryId || !appScopedKey || !orgScopedSubkeyId) + throw new Error('Delivery retry subkey prerequisites were not created') + + const response = await fetch(`${BASE_URL}/webhooks/deliveries/retry`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'x-api-key': appScopedKey, + 'x-limited-key-id': String(orgScopedSubkeyId), + }, + body: JSON.stringify({ + orgId: WEBHOOK_TEST_ORG_ID, + deliveryId: lastDeliveryId, + }), + }) + + expect(response.status).toBe(400) + const data = await response.json() as { error: string, message: string } + expect(data.error).toBe('no_permission') + expect(data.message).toContain('App-scoped API keys') + }) + it('retry delivery with missing body', async () => { const response = await fetch(`${BASE_URL}/webhooks/deliveries/retry`, { method: 'POST',