fix(access-control): exempt legacy blocks (#5063)

This commit is contained in:
Vikhyath Mondreti
2026-06-15 12:50:16 -07:00
committed by GitHub
parent 67e02fab3a
commit 324299eef0
6 changed files with 75 additions and 9 deletions
@@ -27,6 +27,7 @@ import {
import { ArrowLeft } from '@/components/emcn/icons'
import { getEnv, isTruthy } from '@/lib/core/config/env'
import { cn } from '@/lib/core/utils/cn'
import { isBlockTypeAccessControlExempt } from '@/lib/permission-groups/block-access'
import type { PermissionGroupConfig } from '@/lib/permission-groups/types'
import { getUserColor } from '@/lib/workspaces/colors'
import { getAllBlocks } from '@/blocks'
@@ -632,7 +633,7 @@ export function AccessControl() {
}, [viewingGroup, editingConfig])
const allBlocks = useMemo(() => {
const blocks = getAllBlocks().filter((b) => !b.hideFromToolbar && b.type !== 'start_trigger')
const blocks = getAllBlocks().filter((b) => !isBlockTypeAccessControlExempt(b.type))
return blocks.sort((a, b) => {
const categoryOrder = { triggers: 0, blocks: 1, tools: 2 }
const catA = categoryOrder[a.category] ?? 3
@@ -9,6 +9,7 @@ const {
mockIsOrganizationOnEnterprisePlan,
mockGetWorkspaceWithOwner,
mockGetProviderFromModel,
mockGetBlock,
mockExplicitGroup,
mockDefaultGroup,
} = vi.hoisted(() => ({
@@ -40,6 +41,7 @@ const {
mockIsOrganizationOnEnterprisePlan: vi.fn<() => Promise<boolean>>(),
mockGetWorkspaceWithOwner: vi.fn<() => Promise<{ organizationId: string | null } | null>>(),
mockGetProviderFromModel: vi.fn<(model: string) => string>(),
mockGetBlock: vi.fn<(type: string) => { hideFromToolbar?: boolean } | undefined>(),
// The explicit-group query joins permission_group_member -> permission_group;
// the org-default query selects permission_group directly. The db mock returns
// the explicit rows when `innerJoin` was called and the default rows otherwise.
@@ -108,6 +110,11 @@ vi.mock('@/providers/utils', () => ({
getProviderFromModel: mockGetProviderFromModel,
}))
vi.mock('@/blocks/registry', () => ({
getBlock: mockGetBlock,
getAllBlocks: vi.fn(() => []),
}))
import {
assertPermissionsAllowed,
CustomToolsNotAllowedError,
@@ -128,6 +135,15 @@ function setEnterpriseOrgWorkspace() {
mockIsOrganizationOnEnterprisePlan.mockResolvedValue(true)
}
/**
* Default every block to non-legacy. `vi.clearAllMocks()` (used by the
* describe-level hooks) keeps implementations, so reset here to stop a legacy
* `getBlock` implementation set in one test from leaking into later ones.
*/
beforeEach(() => {
mockGetBlock.mockImplementation(() => undefined)
})
describe('IntegrationNotAllowedError', () => {
it.concurrent('creates error with correct name and message', () => {
const error = new IntegrationNotAllowedError('discord')
@@ -280,6 +296,14 @@ describe('validateBlockType', () => {
await validateBlockType(undefined, undefined, 'start_trigger')
})
it('always allows legacy blocks hidden from the toolbar', async () => {
mockGetBlock.mockImplementation((type) =>
type === 'notion' ? { hideFromToolbar: true } : undefined
)
await validateBlockType(undefined, undefined, 'notion')
})
it('matches case-insensitively', async () => {
await validateBlockType(undefined, undefined, 'Slack')
await validateBlockType(undefined, undefined, 'GOOGLE_DRIVE')
@@ -445,6 +469,19 @@ describe('assertPermissionsAllowed', () => {
).rejects.toBeInstanceOf(IntegrationNotAllowedError)
})
it('exempts legacy blocks from the integration allowlist', async () => {
mockExplicitGroup.value = [{ config: { allowedIntegrations: ['slack'] } }]
mockGetBlock.mockImplementation((type) =>
type === 'notion' ? { hideFromToolbar: true } : undefined
)
await assertPermissionsAllowed({
userId: 'user-123',
workspaceId: 'workspace-1',
blockType: 'notion',
})
})
it('throws CustomToolsNotAllowedError when custom tools are disabled', async () => {
mockExplicitGroup.value = [{ config: { disableCustomTools: true } }]
@@ -10,6 +10,7 @@ import {
isInvitationsDisabled,
isPublicApiDisabled,
} from '@/lib/core/config/feature-flags'
import { isBlockTypeAccessControlExempt } from '@/lib/permission-groups/block-access'
import {
DEFAULT_PERMISSION_GROUP_CONFIG,
type PermissionGroupConfig,
@@ -287,7 +288,7 @@ export async function validateBlockType(
blockType: string,
ctx?: ExecutionContext
): Promise<void> {
if (blockType === 'start_trigger') {
if (isBlockTypeAccessControlExempt(blockType)) {
return
}
@@ -478,10 +479,10 @@ interface PermissionAssertion {
export async function assertPermissionsAllowed(req: PermissionAssertion): Promise<void> {
const { userId, workspaceId, model, blockType, toolKind, ctx } = req
if (blockType === 'start_trigger') {
if (!model && !toolKind) {
return
}
const blockTypeExempt = blockType ? isBlockTypeAccessControlExempt(blockType) : false
if (blockTypeExempt && !model && !toolKind) {
return
}
const config =
@@ -509,7 +510,7 @@ export async function assertPermissionsAllowed(req: PermissionAssertion): Promis
}
}
if (blockType && blockType !== 'start_trigger') {
if (blockType && !blockTypeExempt) {
if (config && config.allowedIntegrations !== null) {
if (!config.allowedIntegrations.includes(blockType.toLowerCase())) {
const envAllowlist = getAllowedIntegrationsFromEnv()
+3 -2
View File
@@ -7,6 +7,7 @@ import { ApiClientError } from '@/lib/api/client/errors'
import { requestJson } from '@/lib/api/client/request'
import { getAllowedIntegrationsContract } from '@/lib/api/contracts/common'
import { getEnv, isTruthy } from '@/lib/core/config/env'
import { isBlockTypeAccessControlExempt } from '@/lib/permission-groups/block-access'
import {
DEFAULT_PERMISSION_GROUP_CONFIG,
type PermissionGroupConfig,
@@ -91,7 +92,7 @@ export function usePermissionConfig(): PermissionConfigResult {
const isBlockAllowed = useMemo(() => {
return (blockType: string) => {
if (blockType === 'start_trigger') return true
if (isBlockTypeAccessControlExempt(blockType)) return true
if (mergedAllowedIntegrations === null) return true
return mergedAllowedIntegrations.includes(blockType.toLowerCase())
}
@@ -117,7 +118,7 @@ export function usePermissionConfig(): PermissionConfigResult {
if (mergedAllowedIntegrations === null) return blocks
return blocks.filter(
(block) =>
block.type === 'start_trigger' ||
isBlockTypeAccessControlExempt(block.type) ||
mergedAllowedIntegrations.includes(block.type.toLowerCase())
)
}
@@ -1,6 +1,7 @@
import { createLogger } from '@sim/logger'
import { toError } from '@sim/utils/errors'
import { validateSelectorIds } from '@/lib/copilot/validation/selector-validator'
import { isBlockTypeAccessControlExempt } from '@/lib/permission-groups/block-access'
import type { PermissionGroupConfig } from '@/lib/permission-groups/types'
import {
buildCanonicalIndex,
@@ -714,6 +715,9 @@ export function isBlockTypeAllowed(
blockType: string,
permissionConfig: PermissionGroupConfig | null
): boolean {
if (isBlockTypeAccessControlExempt(blockType)) {
return true
}
if (!permissionConfig || permissionConfig.allowedIntegrations === null) {
return true
}
@@ -0,0 +1,22 @@
import { getBlock } from '@/blocks/registry'
/**
* Block types that bypass permission-group access control entirely.
*
* Two kinds of blocks are exempt:
* - `start_trigger`: the universal workflow entry point. A workflow must always
* be startable regardless of the configured integration allowlist.
* - Legacy blocks (`hideFromToolbar: true`): superseded integration versions and
* deprecated blocks. They never appear in the toolbar or the Access Control
* admin list, so admins cannot allowlist them — yet they may still live inside
* older workflows. Exempting them keeps those workflows runnable instead of
* silently blocking blocks the admin had no way to permit.
*
* This is the single source of truth shared by both the runtime enforcement
* paths and the Access Control admin UI so the "hidden from the list" set and
* the "skipped by enforcement" set never drift apart.
*/
export function isBlockTypeAccessControlExempt(blockType: string): boolean {
if (blockType === 'start_trigger') return true
return getBlock(blockType)?.hideFromToolbar === true
}