fix(rbac): stop mapping TUS auth DB failures to 403 insufficient_permissions

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 @@
   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<string, unknown>)[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<MiddlewareKeyVariables>,
+  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 @@
     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 @@
     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')
   }
 }
 

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<string, unknown> | 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' },
+    })
+  })
+})