Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 68 additions & 26 deletions apps/sim/ee/access-control/utils/permission-check.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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
}

/**
Expand Down Expand Up @@ -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
}

/**
Expand All @@ -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
Expand All @@ -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 }
Expand Down
187 changes: 186 additions & 1 deletion apps/sim/ee/access-control/utils/permission-gate-subject.test.ts
Original file line number Diff line number Diff line change
@@ -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(() => ({
Expand All @@ -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',
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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)
})
})
Loading
Loading