fix(executor): load permission config once per run and retry transient reads (#7847)

* fix(executor): load permission config once per run and retry transient reads

Every block ran on a shallow copy of the execution context, so the permission-config memo written onto it was discarded and the full permission-group config reloaded from the database before every block. That per-block load had no retry, so a single transient database error failed the whole run, and the raw query error (SQL and bound parameters) surfaced as the block error.

- Memoize the in-flight load in a run-scoped map shared by every block copy, keyed by governed subject and workspace; failed loads are evicted
- Move the bounded transient-read retry from the tool-only wrapper into the shared loader so block, model, agent and tool gates all get it
- Replace a database query error's message in the block error handler and log only its redacted cause
- Redact bound parameters in the execution failure cause log

* fix(executor): honor caller cancellation in unshared permission loads

Author: waleedlatif1

apps/sim/ee/access-control/utils/permission-check.ts

@@ -1,10 +1,15 @@
 import { createLogger } from '@sim/logger'
+import { describeError } from '@sim/utils/errors'
+import { sleep } from '@sim/utils/helpers'
+import { backoffWithJitter } from '@sim/utils/retry'
 import type { ShareAuthType } from '@/lib/api/contracts/public-shares'
 import {
   getAllowedIntegrationsFromEnv,
   isInvitationsDisabled,
   isPublicApiDisabled,
 } from '@/lib/core/config/env-flags'
+import { findDatabaseQueryError } from '@/lib/core/errors/database-query-error'
+import { isRetryableInfrastructureError } from '@/lib/core/errors/retryable-infrastructure'
 import { isBlockTypeAccessControlExempt } from '@/lib/permission-groups/block-access'
 import {
   CAPABILITY_RULES,
@@ -199,43 +204,78 @@ function governedSubjectUserId(
   return declared ?? undefined
 }
 
+const PERMISSION_CONFIG_LOAD_MAX_ATTEMPTS = 3
+const PERMISSION_CONFIG_LOAD_RETRY_BACKOFF = { baseMs: 25, maxMs: 100 } as const
+
 /**
- * Cache-aware wrapper around `getUserPermissionConfig`. When an
- * `ExecutionContext` is provided, the resolved config is memoized on the
- * context so repeated checks during a single workflow run share one DB hit.
- *
- * The subject is resolved HERE rather than by each caller, because the memo is
- * keyed by nothing but the context. `validateModelProvider` and
- * `validateBlockType` take the actor's id positionally, so a run declaring a
- * different gate subject had the first model check fill the cache with the
- * BILLING actor's group — and every later `assertPermissionsAllowed`, having
- * correctly resolved the governed subject, was handed that stale entry. Doing
- * the derivation at the one place the config is loaded makes the memo correct
- * by construction: within a run `capabilityGovernedUserId` is fixed, so every
- * path resolves and caches the same person.
+ * Loads a permission config, retrying a transient database read failure a bounded number of times.
+ * The last failure is rethrown: resolving `null` would turn every gate off.
+ */
+async function loadPermissionConfig(
+  userId: string,
+  workspaceId: string,
+  signal: AbortSignal | undefined
+): Promise<PermissionGroupConfig | null> {
+  for (let attempt = 1; ; attempt += 1) {
+    signal?.throwIfAborted()
+    try {
+      return await getUserPermissionConfig(userId, workspaceId)
+    } catch (error) {
+      signal?.throwIfAborted()
+      if (
+        attempt >= PERMISSION_CONFIG_LOAD_MAX_ATTEMPTS ||
+        !findDatabaseQueryError(error) ||
+        !isRetryableInfrastructureError(error)
+      ) {
+        throw error
+      }
+
+      const delayMs = backoffWithJitter(attempt, null, PERMISSION_CONFIG_LOAD_RETRY_BACKOFF)
+      logger.warn('Retrying permission config load after database error', {
+        workspaceId,
+        attempt,
+        maxAttempts: PERMISSION_CONFIG_LOAD_MAX_ATTEMPTS,
+        delayMs,
+        cause: describeError(error),
+      })
+      await sleep(delayMs)
+    }
+  }
+}
+
+/**
+ * Loads the governed subject's permission config. The subject is resolved here, not by callers,
+ * so every gate reads the same person's group. On a run context the in-flight load is memoized per
+ * subject and workspace in the run's `permissionConfigCache`, and a failed load is evicted. A shared
+ * load observes only the run's abort signal, so one caller's cancellation cannot fail it for others;
+ * an unshared load observes the caller's `signal`.
  */
 async function getPermissionConfig(
   actorUserId: string | undefined,
   workspaceId: string | undefined,
-  ctx?: ExecutionContext
+  ctx?: ExecutionContext,
+  signal?: AbortSignal
 ): Promise<PermissionGroupConfig | null> {
   const userId = governedSubjectUserId(actorUserId, ctx)
   if (!userId || !workspaceId) {
     return mergeEnvAllowlist(null)
   }
 
-  if (ctx) {
-    if (ctx.permissionConfigLoaded) {
-      return ctx.permissionConfig ?? null
-    }
-
-    const config = await getUserPermissionConfig(userId, workspaceId)
-    ctx.permissionConfig = config
-    ctx.permissionConfigLoaded = true
-    return config
+  const cache = ctx?.permissionConfigCache
+  if (!cache) {
+    return loadPermissionConfig(userId, workspaceId, signal ?? ctx?.abortSignal)
   }
 
-  return getUserPermissionConfig(userId, workspaceId)
+  const key = `${userId}:${workspaceId}`
+  const cached = cache.get(key)
+  if (cached) return cached
+
+  const pending = loadPermissionConfig(userId, workspaceId, ctx?.abortSignal)
+  cache.set(key, pending)
+  pending.catch(() => {
+    if (cache.get(key) === pending) cache.delete(key)
+  })
+  return pending
 }
 
 /**
@@ -499,6 +539,8 @@ interface PermissionAssertion {
   toolId?: string
   toolKind?: ToolKind
   ctx?: ExecutionContext
+  /** Caller cancellation, observed while loading a config that is not shared through a run cache. */
+  signal?: AbortSignal
 }
 
 /**
@@ -516,7 +558,7 @@ interface PermissionAssertion {
 /** permission-group-enforced: custom_tools.use — gates tool invocation during a run, not an operation */
 /** permission-group-enforced: skills.use — gates skill loading during a run, not an operation */
 export async function assertPermissionsAllowed(req: PermissionAssertion): Promise<void> {
-  const { workspaceId, model, blockType, toolId, toolKind, ctx } = req
+  const { workspaceId, model, blockType, toolId, toolKind, ctx, signal } = req
   const userId = governedSubjectUserId(req.userId, ctx)
 
   const blockTypeExempt = blockType ? isBlockTypeAccessControlExempt(blockType) : false
@@ -527,7 +569,7 @@ export async function assertPermissionsAllowed(req: PermissionAssertion): Promis
 
   const config =
     userId && workspaceId
-      ? await getPermissionConfig(userId, workspaceId, ctx)
+      ? await getPermissionConfig(userId, workspaceId, ctx, signal)
       : mergeEnvAllowlist(null)
 
   const subject = { userId, workspaceId }

apps/sim/ee/access-control/utils/permission-gate-subject.test.ts

@@ -1,6 +1,7 @@
 /**
  * @vitest-environment node
  */
+import { DrizzleQueryError } from 'drizzle-orm/errors'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 const mocks = vi.hoisted(() => ({
@@ -18,6 +19,7 @@ vi.mock('@/lib/billing/core/subscription', () => ({
   isOrganizationOnEnterprisePlan: vi.fn(),
 }))
 vi.mock('@/lib/workspaces/permissions/utils', () => ({ getWorkspaceWithOwner: vi.fn() }))
+vi.mock('@sim/utils/helpers', () => ({ sleep: vi.fn().mockResolvedValue(undefined) }))
 vi.mock('@/providers/utils', () => ({
   isFunctionToolCall: () => false,
   getProviderFromModel: () => 'openai',
@@ -36,7 +38,10 @@ import {
  * field and keeps gating on the caller.
  */
 function runDeclaring(capabilityGovernedUserId?: string | null): ExecutionContext {
-  return { metadata: { capabilityGovernedUserId } } as unknown as ExecutionContext
+  return {
+    metadata: { capabilityGovernedUserId },
+    permissionConfigCache: new Map(),
+  } as unknown as ExecutionContext
 }
 
 describe('the subject a run’s permission gate is decided about', () => {
@@ -164,3 +169,183 @@ describe('the group a run’s later gates read from its cache', () => {
     expect(mocks.getUserPermissionConfig).not.toHaveBeenCalled()
   })
 })
+
+function databaseError(code = 'ECONNRESET'): DrizzleQueryError {
+  return new DrizzleQueryError(
+    'select "billing_blocked" from "user_stats" where "user_stats"."user_id" = $1',
+    ['owner-secret-id'],
+    Object.assign(new Error(`driver failure ${code}`), { code })
+  )
+}
+
+/** Every block runs on a shallow copy of the run's context, so the memo lives in a Map they share. */
+describe('the run-scoped permission config cache', () => {
+  function runContext(overrides: Partial<ExecutionContext> = {}): ExecutionContext {
+    return {
+      metadata: {},
+      permissionConfigCache: new Map(),
+      ...overrides,
+    } as unknown as ExecutionContext
+  }
+
+  function gate(ctx: ExecutionContext, workspaceId = 'workspace-1') {
+    return assertPermissionsAllowed({
+      userId: 'user-1',
+      workspaceId,
+      toolId: 'http_request',
+      ctx,
+    })
+  }
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mocks.getUserPermissionConfig.mockResolvedValue({ deniedTools: [] })
+  })
+
+  it('loads once across the per-block copies of one run', async () => {
+    const run = runContext()
+
+    await gate({ ...run })
+    await gate({ ...run })
+
+    expect(mocks.getUserPermissionConfig).toHaveBeenCalledExactlyOnceWith('user-1', 'workspace-1')
+  })
+
+  it('shares one in-flight load between concurrent parallel branches', async () => {
+    const run = runContext()
+    let release!: (config: unknown) => void
+    mocks.getUserPermissionConfig.mockReturnValueOnce(
+      new Promise((resolve) => {
+        release = resolve
+      })
+    )
+
+    const branches = Promise.all(Array.from({ length: 5 }, () => gate({ ...run })))
+    release({ deniedTools: [] })
+    await branches
+
+    expect(mocks.getUserPermissionConfig).toHaveBeenCalledTimes(1)
+  })
+
+  it('keeps a separate entry per workspace', async () => {
+    const run = runContext()
+    mocks.getUserPermissionConfig.mockImplementation(async (_userId, workspaceId) =>
+      workspaceId === 'workspace-2' ? { deniedTools: ['http_request'] } : { deniedTools: [] }
+    )
+
+    await gate({ ...run }, 'workspace-1')
+    await expect(gate({ ...run }, 'workspace-2')).rejects.toBeInstanceOf(ToolNotAllowedError)
+
+    expect(mocks.getUserPermissionConfig).toHaveBeenCalledTimes(2)
+  })
+
+  it('evicts a failed load so a later gate loads again', async () => {
+    const run = runContext()
+    mocks.getUserPermissionConfig.mockRejectedValueOnce(new Error('config unavailable'))
+
+    await expect(gate({ ...run })).rejects.toThrow('config unavailable')
+    await gate({ ...run })
+
+    expect(mocks.getUserPermissionConfig).toHaveBeenCalledTimes(2)
+  })
+
+  it('retries a transient database failure and then caches the result', async () => {
+    const run = runContext()
+    mocks.getUserPermissionConfig.mockRejectedValueOnce(databaseError())
+
+    await gate({ ...run })
+    await gate({ ...run })
+
+    expect(mocks.getUserPermissionConfig).toHaveBeenCalledTimes(2)
+  })
+
+  it('does not retry a database failure that is not transient', async () => {
+    const sqlError = databaseError('42703')
+    mocks.getUserPermissionConfig.mockRejectedValue(sqlError)
+
+    await expect(gate(runContext())).rejects.toBe(sqlError)
+    expect(mocks.getUserPermissionConfig).toHaveBeenCalledTimes(1)
+  })
+
+  it('fails closed with the last error once retries are exhausted', async () => {
+    const error = databaseError()
+    mocks.getUserPermissionConfig.mockRejectedValue(error)
+
+    await expect(gate(runContext())).rejects.toBe(error)
+    expect(mocks.getUserPermissionConfig).toHaveBeenCalledTimes(3)
+  })
+
+  it('stops retrying when the run is cancelled', async () => {
+    const controller = new AbortController()
+    const reason = new Error('Execution cancelled')
+    mocks.getUserPermissionConfig.mockImplementationOnce(async () => {
+      controller.abort(reason)
+      throw databaseError()
+    })
+
+    await expect(gate(runContext({ abortSignal: controller.signal }))).rejects.toBe(reason)
+    expect(mocks.getUserPermissionConfig).toHaveBeenCalledTimes(1)
+  })
+
+  it('does not memoize on a context that carries no run cache', async () => {
+    const ctx = { metadata: {} } as unknown as ExecutionContext
+
+    await gate(ctx)
+    await gate(ctx)
+
+    expect(mocks.getUserPermissionConfig).toHaveBeenCalledTimes(2)
+    expect(ctx.permissionConfigCache).toBeUndefined()
+  })
+
+  it('stops retrying when the caller of a check outside a run cancels', async () => {
+    const controller = new AbortController()
+    const reason = new Error('Tool cancelled')
+    mocks.getUserPermissionConfig.mockImplementationOnce(async () => {
+      controller.abort(reason)
+      throw databaseError()
+    })
+
+    await expect(
+      assertPermissionsAllowed({
+        userId: 'user-1',
+        workspaceId: 'workspace-1',
+        toolId: 'http_request',
+        signal: controller.signal,
+      })
+    ).rejects.toBe(reason)
+    expect(mocks.getUserPermissionConfig).toHaveBeenCalledTimes(1)
+  })
+
+  it('does not let one caller cancel a load shared through the run cache', async () => {
+    const run = runContext()
+    const controller = new AbortController()
+    mocks.getUserPermissionConfig.mockImplementationOnce(async () => {
+      controller.abort(new Error('Tool cancelled'))
+      throw databaseError()
+    })
+
+    const cancelled = assertPermissionsAllowed({
+      userId: 'user-1',
+      workspaceId: 'workspace-1',
+      toolId: 'http_request',
+      ctx: { ...run },
+      signal: controller.signal,
+    })
+    const other = gate({ ...run })
+
+    await expect(Promise.all([cancelled, other])).resolves.toBeDefined()
+    expect(mocks.getUserPermissionConfig).toHaveBeenCalledTimes(2)
+  })
+
+  it('retries a transient failure for a check made outside a run', async () => {
+    mocks.getUserPermissionConfig.mockRejectedValueOnce(databaseError())
+
+    await assertPermissionsAllowed({
+      userId: 'user-1',
+      workspaceId: 'workspace-1',
+      toolId: 'http_request',
+    })
+
+    expect(mocks.getUserPermissionConfig).toHaveBeenCalledTimes(2)
+  })
+})