diff --git a/supabase/functions/_backend/utils/rbac.ts b/supabase/functions/_backend/utils/rbac.ts index df83e20d8a..cd1bc40e23 100644 --- a/supabase/functions/_backend/utils/rbac.ts +++ b/supabase/functions/_backend/utils/rbac.ts @@ -18,6 +18,8 @@ import type { Context } from 'hono' import type { MiddlewareKeyVariables } from './hono.ts' import { sql } from 'drizzle-orm' +import { HTTPException } from 'hono/http-exception' +import { quickError } from './hono.ts' import { cloudlog, cloudlogErr } from './logging.ts' import { closeClient, getDrizzleClient, getPgClient } from './pg.ts' @@ -87,6 +89,122 @@ export interface PermissionScope { channelId?: number } +const TRANSIENT_NODE_ERROR_CODES = new Set([ + 'ECONNREFUSED', + 'ECONNRESET', + 'ECONNABORTED', + 'ETIMEDOUT', + 'ENETUNREACH', + 'EHOSTUNREACH', + 'EPIPE', + 'EAI_AGAIN', + 'ENOTFOUND', +]) + +// Postgres SQLSTATE classes/codes that mean the connection itself failed. +const TRANSIENT_PG_SQLSTATES = new Set([ + '08000', // connection_exception + '08001', // sqlclient_unable_to_establish_sqlconnection + '08003', // connection_does_not_exist + '08004', // sqlserver_rejected_establishment_of_sqlconnection + '08006', // connection_failure + '08007', // transaction_resolution_unknown + '08P01', // protocol_violation + '57P01', // admin_shutdown + '57P02', // crash_shutdown + '57P03', // cannot_connect_now + '53300', // too_many_connections + '53400', // configuration_limit_exceeded + '57014', // query_canceled (statement_timeout / lock_timeout) +]) + +const TRANSIENT_ERROR_MESSAGE_RE = /connection (?:terminated|ended|closed|refused|reset)|timeout exceeded when trying to connect|connect(?:ion)? timed? ?out|canceling statement due to (?:statement|lock) timeout|network(?: |_)?error|socket hang up|hyperdrive|too many clients already/i + +function readErrorField(error: unknown, key: string): unknown { + if (!error || typeof error !== 'object') + return undefined + return (error as Record)[key] +} + +/** + * Walk Drizzle/node-postgres cause chains for connection/timeout signals. + * Invalid query params (e.g. bad UUID cast 22P02) are NOT transient — those + * must keep looking like ACL deny so authz endpoints stay stable. + */ +function isTransientPermissionCheckError(error: unknown, depth = 0): boolean { + if (!error || depth > 6) + return false + + if (typeof error === 'string') + return TRANSIENT_ERROR_MESSAGE_RE.test(error) + + const code = readErrorField(error, 'code') + if (typeof code === 'string') { + if (TRANSIENT_NODE_ERROR_CODES.has(code) || TRANSIENT_PG_SQLSTATES.has(code)) + return true + } + + const message = readErrorField(error, 'message') + if (typeof message === 'string' && TRANSIENT_ERROR_MESSAGE_RE.test(message)) + return true + + const errno = readErrorField(error, 'errno') + if (typeof errno === 'string' && TRANSIENT_NODE_ERROR_CODES.has(errno)) + return true + + const cause = readErrorField(error, 'cause') + if (cause !== undefined && isTransientPermissionCheckError(cause, depth + 1)) + return true + + const errors = readErrorField(error, 'errors') + if (Array.isArray(errors)) { + return errors.some(entry => isTransientPermissionCheckError(entry, depth + 1)) + } + + return false +} + +/** + * Permission helpers must never map infrastructure failures to "denied". + * Callers treat `false` as ACL deny (401/403). Transient Hyperdrive/Postgres + * errors must surface as 503 so clients (especially TUS) can retry. + * Non-transient query failures (invalid UUID, etc.) still deny. + */ +function handlePermissionCheckError( + c: Context, + permission: Permission, + scope: PermissionScope, + error: unknown, + source: 'checkPermission' | 'checkPermissionPg', +): false { + if (error instanceof HTTPException) + throw error + + const transient = isTransientPermissionCheckError(error) + + cloudlogErr({ + requestId: c.get('requestId'), + message: `${source} error`, + error, + permission, + scope, + transient, + }) + + if (transient) { + quickError( + 503, + 'upstream_unavailable', + 'Permission check temporarily unavailable', + { permission, scope }, + error, + { alert: false }, + ) + } + + return false +} + // ============================================================================= // Core Functions // ============================================================================= @@ -205,14 +323,7 @@ export async function checkPermission( return allowed } catch (e) { - cloudlogErr({ - requestId: c.get('requestId'), - message: 'checkPermission error', - error: e, - permission, - scope, - }) - return false + return handlePermissionCheckError(c, permission, scope, e, 'checkPermission') } finally { if (pgClient) { @@ -367,14 +478,7 @@ export async function checkPermissionPg( return allowed } catch (e) { - cloudlogErr({ - requestId: c.get('requestId'), - message: 'checkPermissionPg error', - error: e, - permission, - scope, - }) - return false + return handlePermissionCheckError(c, permission, scope, e, 'checkPermissionPg') } } diff --git a/tests/rbac-permission-infra-errors.unit.test.ts b/tests/rbac-permission-infra-errors.unit.test.ts new file mode 100644 index 0000000000..0f5bc7aade --- /dev/null +++ b/tests/rbac-permission-infra-errors.unit.test.ts @@ -0,0 +1,229 @@ +import { HTTPException } from 'hono/http-exception' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const executeMock = vi.fn() +const getPgClientMock = vi.fn(() => ({})) +const getDrizzleClientMock = vi.fn(() => ({ + execute: executeMock, +})) +const closeClientMock = vi.fn() + +vi.mock('../supabase/functions/_backend/utils/logging.ts', () => ({ + cloudlog: vi.fn(), + cloudlogErr: vi.fn(), +})) + +vi.mock('../supabase/functions/_backend/utils/pg.ts', () => ({ + closeClient: closeClientMock, + getDrizzleClient: getDrizzleClientMock, + getPgClient: getPgClientMock, +})) + +const { checkPermission, checkPermissionPg } = await import('../supabase/functions/_backend/utils/rbac.ts') + +function makeContext(auth: Record | undefined = { + userId: '00000000-0000-4000-8000-000000000001', + authType: 'apikey', + apikey: { + key: 'capgo_test_key', + rbac_id: 42, + }, +}) { + return { + get: (key: string) => { + if (key === 'auth') + return auth + if (key === 'requestId') + return 'req-test' + if (key === 'capgkey') + return 'capgo_test_key' + return undefined + }, + } as any +} + +describe('rbac permission infra errors', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('checkPermission returns false for real ACL denials', async () => { + executeMock.mockResolvedValueOnce({ rows: [{ allowed: false }] }) + + await expect(checkPermission(makeContext(), 'app.upload_bundle', { appId: 'ai.offthetools.app' })) + .resolves + .toBe(false) + }) + + it('checkPermission surfaces connection failures as 503 upstream_unavailable', async () => { + executeMock.mockRejectedValueOnce(Object.assign(new Error('Connection terminated unexpectedly'), { + code: 'ECONNRESET', + })) + + await expect(checkPermission(makeContext(), 'app.upload_bundle', { appId: 'ai.offthetools.app' })) + .rejects + .toMatchObject({ + status: 503, + cause: { + error: 'upstream_unavailable', + message: 'Permission check temporarily unavailable', + }, + }) + + expect(closeClientMock).toHaveBeenCalled() + }) + + it('checkPermissionPg surfaces Hyperdrive connect timeouts as 503', async () => { + executeMock.mockRejectedValueOnce(new Error('timeout exceeded when trying to connect')) + + await expect(checkPermissionPg( + makeContext(), + 'app.upload_bundle', + { appId: 'ai.offthetools.app' }, + getDrizzleClientMock() as any, + '00000000-0000-4000-8000-000000000001', + 'capgo_test_key', + )).rejects.toMatchObject({ + status: 503, + cause: { + error: 'upstream_unavailable', + }, + }) + }) + + it('checkPermissionPg treats invalid UUID cast errors as ACL deny, not 503', async () => { + const invalidUuidError = Object.assign(new Error('invalid input syntax for type uuid: "non-user-org-id"'), { + code: '22P02', + }) + executeMock.mockRejectedValueOnce(invalidUuidError) + + await expect(checkPermissionPg( + makeContext(), + 'org.read', + { orgId: 'non-user-org-id' }, + getDrizzleClientMock() as any, + '00000000-0000-4000-8000-000000000001', + 'capgo_test_key', + )).resolves.toBe(false) + }) + + it('checkPermission treats Drizzle-wrapped invalid UUID cast as ACL deny', async () => { + const drizzleWrapped = Object.assign(new Error('Failed query: SELECT ...'), { + name: 'DrizzleQueryError', + cause: Object.assign(new Error('invalid input syntax for type uuid: "046a...-missing"'), { + code: '22P02', + }), + }) + executeMock.mockRejectedValueOnce(drizzleWrapped) + + await expect(checkPermission(makeContext(), 'org.invite_user', { orgId: '046a36ac-e03c-4590-9257-bd6c9dba9ee8-missing' })) + .resolves + .toBe(false) + }) + + it('checkPermissionPg unwraps Drizzle cause for transient PG connection codes', async () => { + const drizzleWrapped = Object.assign(new Error('Failed query: SELECT ...'), { + name: 'DrizzleQueryError', + cause: Object.assign(new Error('terminating connection due to administrator command'), { + code: '57P01', + }), + }) + executeMock.mockRejectedValueOnce(drizzleWrapped) + + await expect(checkPermissionPg( + makeContext(), + 'app.upload_bundle', + { appId: 'ai.offthetools.app' }, + getDrizzleClientMock() as any, + '00000000-0000-4000-8000-000000000001', + 'capgo_test_key', + )).rejects.toMatchObject({ + status: 503, + cause: { error: 'upstream_unavailable' }, + }) + }) + + it('checkPermissionPg rethrows existing HTTPException without remapping', async () => { + const httpError = new HTTPException(409, { message: 'conflict' }) + executeMock.mockRejectedValueOnce(httpError) + + await expect(checkPermissionPg( + makeContext(), + 'app.upload_bundle', + { appId: 'ai.offthetools.app' }, + getDrizzleClientMock() as any, + '00000000-0000-4000-8000-000000000001', + 'capgo_test_key', + )).rejects.toBe(httpError) + }) + + it('checkPermissionPg returns false for real ACL denials', async () => { + executeMock.mockResolvedValueOnce({ rows: [{ allowed: false }] }) + + await expect(checkPermissionPg( + makeContext(), + 'app.upload_bundle', + { appId: 'ai.offthetools.app' }, + getDrizzleClientMock() as any, + '00000000-0000-4000-8000-000000000001', + 'capgo_test_key', + )).resolves.toBe(false) + }) + + it('checkPermission allows JWT auth through the non-rbac_id query path', async () => { + executeMock.mockResolvedValueOnce({ rows: [{ allowed: true }] }) + + await expect(checkPermission( + makeContext({ + userId: '00000000-0000-4000-8000-000000000001', + authType: 'jwt', + apikey: null, + }), + 'app.upload_bundle', + { appId: 'ai.offthetools.app' }, + )).resolves.toBe(true) + }) + + it('checkPermission surfaces statement timeouts on the JWT path as 503', async () => { + executeMock.mockRejectedValueOnce(Object.assign( + new Error('canceling statement due to statement timeout'), + { code: '57014' }, + )) + + await expect(checkPermission( + makeContext({ + userId: '00000000-0000-4000-8000-000000000001', + authType: 'jwt', + apikey: null, + }), + 'org.invite_user', + { orgId: '00000000-0000-4000-8000-000000000099' }, + )).rejects.toMatchObject({ + status: 503, + cause: { error: 'upstream_unavailable' }, + }) + }) + + it('checkPermissionPg surfaces statement timeouts as 503 on the non-rbac_id path', async () => { + executeMock.mockRejectedValueOnce(Object.assign( + new Error('canceling statement due to statement timeout'), + { code: '57014' }, + )) + + await expect(checkPermissionPg( + makeContext({ + userId: '00000000-0000-4000-8000-000000000001', + authType: 'jwt', + apikey: null, + }), + 'app.upload_bundle', + { appId: 'ai.offthetools.app' }, + getDrizzleClientMock() as any, + '00000000-0000-4000-8000-000000000001', + null, + )).rejects.toMatchObject({ + status: 503, + cause: { error: 'upstream_unavailable' }, + }) + }) +}) diff --git a/tests/stats.test.ts b/tests/stats.test.ts index aa4789c2ff..3c52e7b189 100644 --- a/tests/stats.test.ts +++ b/tests/stats.test.ts @@ -4,7 +4,7 @@ import { env } from 'node:process' import { afterAll, beforeAll, describe, expect, it } from 'vitest' import { ALLOWED_STATS_ACTIONS } from '../supabase/functions/_backend/plugin_runtime/plugins/stats_actions.ts' -import { APP_NAME, createAppVersions, getBaseData, getSupabaseClient, getVersionFromAction, headers, ORG_ID, PLUGIN_BASE_URL, resetAndSeedAppData, resetAndSeedAppDataStats, resetAppData, resetAppDataStats, USER_ID } from './test-utils.ts' +import { APP_NAME, createAppVersions, executeSQL, getBaseData, getSupabaseClient, getVersionFromAction, headers, ORG_ID, PLUGIN_BASE_URL, resetAndSeedAppData, resetAndSeedAppDataStats, resetAppData, resetAppDataStats, USER_ID } from './test-utils.ts' const id = randomUUID() const APP_NAME_STATS = `${APP_NAME}.${id}` @@ -884,51 +884,52 @@ describe('rollout trigger metadata', () => { const appId = `${APP_NAME}.rollout.trigger.${shortId}` await resetAndSeedAppData(appId) await resetAndSeedAppDataStats(appId) - const supabase = getSupabaseClient() try { const stableVersion = await createAppVersions(`1.0.0-stable-${shortId}.1`, appId) const rolloutVersion = await createAppVersions(`1.0.0-rollout-${shortId}.1`, appId) - const { data: channel, error: channelError } = await supabase - .from('channels') - .insert({ - app_id: appId, - name: `production-${shortId}`, - version: stableVersion.id, - rollout_version: rolloutVersion.id, - rollout_enabled: true, - rollout_percentage_bps: 5000, - created_by: USER_ID, - owner_org: ORG_ID, - }) - .select('id') - .single() - expect(channelError).toBeNull() - expect(channel).toBeTruthy() + // Bypass PostgREST/Kong for channel seed writes — under CF shard load Kong + // returns "An invalid response was received from the upstream server". + const channelRows = await executeSQL<{ id: number }>( + `INSERT INTO public.channels ( + app_id, name, version, rollout_version, rollout_enabled, + rollout_percentage_bps, created_by, owner_org + ) VALUES ( + $1, $2, $3::bigint, $4::bigint, true, 5000, $5::uuid, $6::uuid + ) + RETURNING id`, + [appId, `production-${shortId}`, stableVersion.id, rolloutVersion.id, USER_ID, ORG_ID], + ) + expect(channelRows[0]?.id).toBeTruthy() const triggeredAt = new Date().toISOString() const reason = `Auto-pause rollback test ${shortId}` - await supabase - .from('channels') - .update({ - rollout_version: null, - rollout_enabled: false, - rollout_percentage_bps: 0, - rollout_paused_at: null, - rollout_pause_reason: reason, - auto_pause_last_triggered_at: triggeredAt, - }) - .eq('id', channel!.id) - .throwOnError() - - const { data: updatedChannel, error: updatedError } = await supabase - .from('channels') - .select('rollout_version, rollout_paused_at, rollout_pause_reason, auto_pause_last_triggered_at') - .eq('id', channel!.id) - .single() + await executeSQL( + `UPDATE public.channels + SET rollout_version = NULL, + rollout_enabled = false, + rollout_percentage_bps = 0, + rollout_paused_at = NULL, + rollout_pause_reason = $2, + auto_pause_last_triggered_at = $3::timestamptz + WHERE id = $1::bigint`, + [channelRows[0]!.id, reason, triggeredAt], + ) + + const updatedRows = await executeSQL<{ + rollout_version: number | null + rollout_paused_at: string | null + rollout_pause_reason: string | null + auto_pause_last_triggered_at: string | null + }>( + `SELECT rollout_version, rollout_paused_at, rollout_pause_reason, auto_pause_last_triggered_at + FROM public.channels + WHERE id = $1::bigint`, + [channelRows[0]!.id], + ) + const updatedChannel = updatedRows[0] - expect(updatedError).toBeNull() expect(updatedChannel?.rollout_version).toBeNull() expect(updatedChannel?.rollout_paused_at).toBeNull() expect(updatedChannel?.rollout_pause_reason).toBe(reason) diff --git a/tests/test-utils.ts b/tests/test-utils.ts index d4008fc1a4..2dc31c47c6 100644 --- a/tests/test-utils.ts +++ b/tests/test-utils.ts @@ -418,7 +418,6 @@ export async function createDirectApiKeyWithBindings(options: { } } - let cachedAuthHeaders: Record | null = null let authHeadersPromise: Promise> | null = null @@ -861,10 +860,10 @@ export async function getPostgresClient(): Promise { return pool } -export async function executeSQL(query: string, params?: any[]): Promise { +export async function executeSQL(query: string, params?: any[]): Promise { const client = await getPostgresClient() const result = await client.query(query, params || []) - return result.rows + return result.rows as T[] } export async function getCronPlanQueueCount(): Promise {