Skip to content

Commit 4eaad05

Browse files
waleedlatif1waleedlatif1
committed
added safety incase userId is missing
1 parent 2d863f9 commit 4eaad05

File tree

7 files changed

+95
-36
lines changed

7 files changed

+95
-36
lines changed

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

Lines changed: 73 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -29,32 +29,35 @@ const mockGetAllowedIntegrationsFromEnv = vi.fn<() => string[] | null>()
2929
const mockIsOrganizationOnEnterprisePlan = vi.fn<() => Promise<boolean>>()
3030
const mockGetProviderFromModel = vi.fn<(model: string) => string>()
3131

32-
vi.doMock('@sim/db', () => databaseMock)
33-
vi.doMock('@sim/db/schema', () => ({}))
34-
vi.doMock('@sim/logger', () => loggerMock)
35-
vi.doMock('drizzle-orm', () => drizzleOrmMock)
36-
vi.doMock('@/lib/billing', () => ({
32+
vi.mock('@sim/db', () => databaseMock)
33+
vi.mock('@sim/db/schema', () => ({}))
34+
vi.mock('@sim/logger', () => loggerMock)
35+
vi.mock('drizzle-orm', () => drizzleOrmMock)
36+
vi.mock('@/lib/billing', () => ({
3737
isOrganizationOnEnterprisePlan: mockIsOrganizationOnEnterprisePlan,
3838
}))
39-
vi.doMock('@/lib/core/config/feature-flags', () => ({
39+
vi.mock('@/lib/core/config/feature-flags', () => ({
4040
getAllowedIntegrationsFromEnv: mockGetAllowedIntegrationsFromEnv,
4141
isAccessControlEnabled: false,
4242
isHosted: false,
4343
}))
44-
vi.doMock('@/lib/permission-groups/types', () => ({
44+
vi.mock('@/lib/permission-groups/types', () => ({
4545
DEFAULT_PERMISSION_GROUP_CONFIG,
4646
parsePermissionGroupConfig: (config: unknown) => {
4747
if (!config || typeof config !== 'object') return DEFAULT_PERMISSION_GROUP_CONFIG
4848
return { ...DEFAULT_PERMISSION_GROUP_CONFIG, ...config }
4949
},
5050
}))
51-
vi.doMock('@/providers/utils', () => ({
51+
vi.mock('@/providers/utils', () => ({
5252
getProviderFromModel: mockGetProviderFromModel,
5353
}))
5454

55-
const { IntegrationNotAllowedError, getUserPermissionConfig, validateBlockType } = await import(
56-
'./permission-check'
57-
)
55+
import { getAllowedIntegrationsFromEnv } from '@/lib/core/config/feature-flags'
56+
import {
57+
getUserPermissionConfig,
58+
IntegrationNotAllowedError,
59+
validateBlockType,
60+
} from './permission-check'
5861

5962
describe('IntegrationNotAllowedError', () => {
6063
it.concurrent('creates error with correct name and message', () => {
@@ -104,6 +107,56 @@ describe('getUserPermissionConfig', () => {
104107
})
105108
})
106109

110+
describe('env allowlist fallback when userId is absent', () => {
111+
beforeEach(() => {
112+
vi.clearAllMocks()
113+
})
114+
115+
it('returns null config when no userId and no env allowlist', async () => {
116+
mockGetAllowedIntegrationsFromEnv.mockReturnValue(null)
117+
118+
const userId: string | undefined = undefined
119+
const permissionConfig = userId ? await getUserPermissionConfig(userId) : null
120+
const allowedIntegrations =
121+
permissionConfig?.allowedIntegrations ?? getAllowedIntegrationsFromEnv()
122+
123+
expect(allowedIntegrations).toBeNull()
124+
})
125+
126+
it('falls back to env allowlist when no userId is provided', async () => {
127+
mockGetAllowedIntegrationsFromEnv.mockReturnValue(['slack', 'gmail'])
128+
129+
const userId: string | undefined = undefined
130+
const permissionConfig = userId ? await getUserPermissionConfig(userId) : null
131+
const allowedIntegrations =
132+
permissionConfig?.allowedIntegrations ?? getAllowedIntegrationsFromEnv()
133+
134+
expect(allowedIntegrations).toEqual(['slack', 'gmail'])
135+
})
136+
137+
it('env allowlist filters block types when userId is absent', async () => {
138+
mockGetAllowedIntegrationsFromEnv.mockReturnValue(['slack', 'gmail'])
139+
140+
const userId: string | undefined = undefined
141+
const permissionConfig = userId ? await getUserPermissionConfig(userId) : null
142+
const allowedIntegrations =
143+
permissionConfig?.allowedIntegrations ?? getAllowedIntegrationsFromEnv()
144+
145+
expect(allowedIntegrations).not.toBeNull()
146+
expect(allowedIntegrations!.includes('slack')).toBe(true)
147+
expect(allowedIntegrations!.includes('discord')).toBe(false)
148+
})
149+
150+
it('uses permission config when userId is present, ignoring env fallback', async () => {
151+
mockGetAllowedIntegrationsFromEnv.mockReturnValue(['slack', 'gmail'])
152+
153+
const config = await getUserPermissionConfig('user-123')
154+
155+
expect(config).not.toBeNull()
156+
expect(config!.allowedIntegrations).toEqual(['slack', 'gmail'])
157+
})
158+
})
159+
107160
describe('validateBlockType', () => {
108161
beforeEach(() => {
109162
vi.clearAllMocks()
@@ -115,15 +168,15 @@ describe('validateBlockType', () => {
115168
})
116169

117170
it('allows any block type', async () => {
118-
await expect(validateBlockType(undefined, 'google_drive')).resolves.not.toThrow()
171+
await validateBlockType(undefined, 'google_drive')
119172
})
120173

121174
it('allows multi-word block types', async () => {
122-
await expect(validateBlockType(undefined, 'microsoft_excel')).resolves.not.toThrow()
175+
await validateBlockType(undefined, 'microsoft_excel')
123176
})
124177

125178
it('always allows start_trigger', async () => {
126-
await expect(validateBlockType(undefined, 'start_trigger')).resolves.not.toThrow()
179+
await validateBlockType(undefined, 'start_trigger')
127180
})
128181
})
129182

@@ -137,9 +190,9 @@ describe('validateBlockType', () => {
137190
})
138191

139192
it('allows block types on the allowlist', async () => {
140-
await expect(validateBlockType(undefined, 'slack')).resolves.not.toThrow()
141-
await expect(validateBlockType(undefined, 'google_drive')).resolves.not.toThrow()
142-
await expect(validateBlockType(undefined, 'microsoft_excel')).resolves.not.toThrow()
193+
await validateBlockType(undefined, 'slack')
194+
await validateBlockType(undefined, 'google_drive')
195+
await validateBlockType(undefined, 'microsoft_excel')
143196
})
144197

145198
it('rejects block types not on the allowlist', async () => {
@@ -149,12 +202,12 @@ describe('validateBlockType', () => {
149202
})
150203

151204
it('always allows start_trigger regardless of allowlist', async () => {
152-
await expect(validateBlockType(undefined, 'start_trigger')).resolves.not.toThrow()
205+
await validateBlockType(undefined, 'start_trigger')
153206
})
154207

155208
it('matches case-insensitively', async () => {
156-
await expect(validateBlockType(undefined, 'Slack')).resolves.not.toThrow()
157-
await expect(validateBlockType(undefined, 'GOOGLE_DRIVE')).resolves.not.toThrow()
209+
await validateBlockType(undefined, 'Slack')
210+
await validateBlockType(undefined, 'GOOGLE_DRIVE')
158211
})
159212

160213
it('includes reason in error for env-only enforcement', async () => {

‎apps/sim/lib/copilot/process-contents.ts‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { db } from '@sim/db'
22
import { copilotChats, document, knowledgeBase, templates } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
44
import { and, eq, isNull } from 'drizzle-orm'
5+
import { getAllowedIntegrationsFromEnv } from '@/lib/core/config/feature-flags'
56
import { loadWorkflowFromNormalizedTables } from '@/lib/workflows/persistence/utils'
67
import { sanitizeForCopilot } from '@/lib/workflows/sanitization/json-sanitizer'
78
import { isHiddenFromDisplay } from '@/blocks/types'
@@ -349,16 +350,14 @@ async function processBlockMetadata(
349350
userId?: string
350351
): Promise<AgentContext | null> {
351352
try {
352-
if (userId) {
353-
const permissionConfig = await getUserPermissionConfig(userId)
354-
const allowedIntegrations = permissionConfig?.allowedIntegrations
355-
if (allowedIntegrations != null && !allowedIntegrations.includes(blockId.toLowerCase())) {
356-
logger.debug('Block not allowed by permission group', { blockId, userId })
357-
return null
358-
}
353+
const permissionConfig = userId ? await getUserPermissionConfig(userId) : null
354+
const allowedIntegrations =
355+
permissionConfig?.allowedIntegrations ?? getAllowedIntegrationsFromEnv()
356+
if (allowedIntegrations != null && !allowedIntegrations.includes(blockId.toLowerCase())) {
357+
logger.debug('Block not allowed by integration allowlist', { blockId, userId })
358+
return null
359359
}
360360

361-
// Reuse registry to match get_blocks_metadata tool result
362361
const { registry: blockRegistry } = await import('@/blocks/registry')
363362
const { tools: toolsRegistry } = await import('@/tools/registry')
364363
const SPECIAL_BLOCKS_METADATA: Record<string, any> = {}
@@ -466,7 +465,6 @@ async function processWorkflowBlockFromDb(
466465
if (!block) return null
467466
const tag = label ? `@${label} in Workflow` : `@${block.name || blockId} in Workflow`
468467

469-
// Build content: isolate the block and include its subBlocks fully
470468
const contentObj = {
471469
workflowId,
472470
block: block,
@@ -518,7 +516,6 @@ async function processExecutionLogFromDb(
518516
endedAt: log.endedAt?.toISOString?.() || (log.endedAt ? String(log.endedAt) : null),
519517
totalDurationMs: log.totalDurationMs ?? null,
520518
workflowName: log.workflowName || '',
521-
// Include trace spans and any available details without being huge
522519
executionData: log.executionData
523520
? {
524521
traceSpans: (log.executionData as any).traceSpans || undefined,

‎apps/sim/lib/copilot/tools/server/blocks/get-block-config.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
GetBlockConfigResult,
77
type GetBlockConfigResultType,
88
} from '@/lib/copilot/tools/shared/schemas'
9+
import { getAllowedIntegrationsFromEnv } from '@/lib/core/config/feature-flags'
910
import { registry as blockRegistry, getLatestBlock } from '@/blocks/registry'
1011
import { isHiddenFromDisplay, type SubBlockConfig } from '@/blocks/types'
1112
import { getUserPermissionConfig } from '@/ee/access-control/utils/permission-check'
@@ -439,7 +440,8 @@ export const getBlockConfigServerTool: BaseServerTool<
439440
}
440441

441442
const permissionConfig = context?.userId ? await getUserPermissionConfig(context.userId) : null
442-
const allowedIntegrations = permissionConfig?.allowedIntegrations
443+
const allowedIntegrations =
444+
permissionConfig?.allowedIntegrations ?? getAllowedIntegrationsFromEnv()
443445

444446
if (allowedIntegrations != null && !allowedIntegrations.includes(blockType.toLowerCase())) {
445447
throw new Error(`Block "${blockType}" is not available`)

‎apps/sim/lib/copilot/tools/server/blocks/get-block-options.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
GetBlockOptionsResult,
77
type GetBlockOptionsResultType,
88
} from '@/lib/copilot/tools/shared/schemas'
9+
import { getAllowedIntegrationsFromEnv } from '@/lib/core/config/feature-flags'
910
import { registry as blockRegistry, getLatestBlock } from '@/blocks/registry'
1011
import { getUserPermissionConfig } from '@/ee/access-control/utils/permission-check'
1112
import { tools as toolsRegistry } from '@/tools/registry'
@@ -59,7 +60,8 @@ export const getBlockOptionsServerTool: BaseServerTool<
5960
}
6061

6162
const permissionConfig = context?.userId ? await getUserPermissionConfig(context.userId) : null
62-
const allowedIntegrations = permissionConfig?.allowedIntegrations
63+
const allowedIntegrations =
64+
permissionConfig?.allowedIntegrations ?? getAllowedIntegrationsFromEnv()
6365

6466
if (allowedIntegrations != null && !allowedIntegrations.includes(blockId.toLowerCase())) {
6567
throw new Error(`Block "${blockId}" is not available`)

‎apps/sim/lib/copilot/tools/server/blocks/get-blocks-and-tools.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import type { BaseServerTool } from '@/lib/copilot/tools/server/base-tool'
33
import { GetBlocksAndToolsInput, GetBlocksAndToolsResult } from '@/lib/copilot/tools/shared/schemas'
4+
import { getAllowedIntegrationsFromEnv } from '@/lib/core/config/feature-flags'
45
import { registry as blockRegistry } from '@/blocks/registry'
56
import type { BlockConfig } from '@/blocks/types'
67
import { getUserPermissionConfig } from '@/ee/access-control/utils/permission-check'
@@ -17,7 +18,8 @@ export const getBlocksAndToolsServerTool: BaseServerTool<
1718
logger.debug('Executing get_blocks_and_tools')
1819

1920
const permissionConfig = context?.userId ? await getUserPermissionConfig(context.userId) : null
20-
const allowedIntegrations = permissionConfig?.allowedIntegrations
21+
const allowedIntegrations =
22+
permissionConfig?.allowedIntegrations ?? getAllowedIntegrationsFromEnv()
2123

2224
type BlockListItem = {
2325
type: string

‎apps/sim/lib/copilot/tools/server/blocks/get-blocks-metadata-tool.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { join } from 'path'
33
import { createLogger } from '@sim/logger'
44
import type { BaseServerTool } from '@/lib/copilot/tools/server/base-tool'
55
import { GetBlocksMetadataInput, GetBlocksMetadataResult } from '@/lib/copilot/tools/shared/schemas'
6+
import { getAllowedIntegrationsFromEnv } from '@/lib/core/config/feature-flags'
67
import { registry as blockRegistry } from '@/blocks/registry'
78
import { AuthMode, type BlockConfig, isHiddenFromDisplay } from '@/blocks/types'
89
import { getUserPermissionConfig } from '@/ee/access-control/utils/permission-check'
@@ -112,7 +113,8 @@ export const getBlocksMetadataServerTool: BaseServerTool<
112113
logger.debug('Executing get_blocks_metadata', { count: blockIds?.length })
113114

114115
const permissionConfig = context?.userId ? await getUserPermissionConfig(context.userId) : null
115-
const allowedIntegrations = permissionConfig?.allowedIntegrations
116+
const allowedIntegrations =
117+
permissionConfig?.allowedIntegrations ?? getAllowedIntegrationsFromEnv()
116118

117119
const result: Record<string, CopilotBlockMetadata> = {}
118120
for (const blockId of blockIds || []) {
@@ -420,7 +422,6 @@ function extractInputs(metadata: CopilotBlockMetadata): {
420422
}
421423

422424
if (schema.options && schema.options.length > 0) {
423-
// Always return the id (actual value to use), not the display label
424425
input.options = schema.options.map((opt) => opt.id || opt.label)
425426
}
426427

‎apps/sim/lib/copilot/tools/server/blocks/get-trigger-blocks.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import { z } from 'zod'
33
import type { BaseServerTool } from '@/lib/copilot/tools/server/base-tool'
4+
import { getAllowedIntegrationsFromEnv } from '@/lib/core/config/feature-flags'
45
import { registry as blockRegistry } from '@/blocks/registry'
56
import type { BlockConfig } from '@/blocks/types'
67
import { getUserPermissionConfig } from '@/ee/access-control/utils/permission-check'
@@ -22,7 +23,8 @@ export const getTriggerBlocksServerTool: BaseServerTool<
2223
logger.debug('Executing get_trigger_blocks')
2324

2425
const permissionConfig = context?.userId ? await getUserPermissionConfig(context.userId) : null
25-
const allowedIntegrations = permissionConfig?.allowedIntegrations
26+
const allowedIntegrations =
27+
permissionConfig?.allowedIntegrations ?? getAllowedIntegrationsFromEnv()
2628

2729
const triggerBlockIds: string[] = []
2830

0 commit comments

Comments
 (0)