Skip to content

Commit 9438339

Browse files
committed
fix(access-control): key the copilot schema cache on permission policy
The deferred integration-tool schemas now depend on the viewer's permission group, but the cache key encoded only identity and block visibility, so an admin's change to deniedTools took effect only when the entry expired. Resolve the config before the key and add a gate signature alongside the existing visibility signature, mirroring how block visibility already keys the same cache. The read moves out of the cached section rather than being added: what the entry caches is a user-tool schema per exposed integration tool, which dominates it.
1 parent 6a03859 commit 9438339

3 files changed

Lines changed: 64 additions & 10 deletions

File tree

apps/sim/lib/copilot/chat/payload.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,34 @@ describe('buildIntegrationToolSchemas', () => {
299299
expect(second[0].input_schema).not.toHaveProperty('mutated')
300300
expect(second[0].outputs).not.toHaveProperty('mutated')
301301
})
302+
303+
it('rebuilds instead of serving a cache entry from the previous policy', async () => {
304+
mockGetHighestPrioritySubscription.mockResolvedValue({ plan: 'pro', status: 'active' })
305+
mockGetUserPermissionConfig.mockResolvedValue({ allowedIntegrations: null, deniedTools: [] })
306+
307+
const before = await buildIntegrationToolSchemas(
308+
'user-policy',
309+
undefined,
310+
{ schemaSurface: 'copilot' },
311+
'workspace-policy'
312+
)
313+
expect(before.map((tool) => tool.name)).toContain('gmail_send')
314+
315+
// An admin denies the tool. The viewer and surface are unchanged, so only
316+
// the policy component of the key can force a rebuild.
317+
mockGetUserPermissionConfig.mockResolvedValue({
318+
allowedIntegrations: null,
319+
deniedTools: ['gmail_send'],
320+
})
321+
322+
const after = await buildIntegrationToolSchemas(
323+
'user-policy',
324+
undefined,
325+
{ schemaSurface: 'copilot' },
326+
'workspace-policy'
327+
)
328+
expect(after.map((tool) => tool.name)).not.toContain('gmail_send')
329+
})
302330
})
303331

304332
describe('buildCopilotRequestPayload', () => {

apps/sim/lib/copilot/chat/payload.ts

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { getBlockVisibilityForCopilot, visibilitySignature } from '@/lib/copilot
99
import type { VfsSnapshotV1 } from '@/lib/copilot/generated/vfs-snapshot-v1'
1010
import {
1111
type IntegrationGateConfig,
12+
integrationGateSignature,
1213
projectIntegrationToolsForViewer,
1314
} from '@/lib/copilot/integration-tool-projection'
1415
import { buildTaggedMcpToolSchemas } from '@/lib/copilot/mcp-tools'
@@ -110,11 +111,14 @@ function getIntegrationToolSchemaCacheKey(
110111
userId: string,
111112
workspaceId: string | undefined,
112113
schemaSurface: string,
113-
visSignature: string
114+
visSignature: string,
115+
gateSignature: string
114116
): string {
115117
// The visibility signature keys the entry to the viewer's gated projection —
116118
// two users in one workspace with different preview reveals must not share.
117-
return JSON.stringify([userId, workspaceId ?? null, schemaSurface, visSignature])
119+
// The gate signature does the same for permission-group policy, so an admin's
120+
// change takes effect on the next build rather than when the entry expires.
121+
return JSON.stringify([userId, workspaceId ?? null, schemaSurface, visSignature, gateSignature])
118122
}
119123

120124
function cloneToolSchemas(toolSchemas: ToolSchema[]): ToolSchema[] {
@@ -151,11 +155,20 @@ export async function buildIntegrationToolSchemas(
151155
): Promise<ToolSchema[]> {
152156
const schemaSurface = options.schemaSurface ?? 'copilot'
153157
const vis = await getBlockVisibilityForCopilot(userId, workspaceId)
158+
// Resolved before the key, not inside the cached build, so the entry is keyed
159+
// to the policy it was produced under. The read this adds is cheap next to
160+
// what the entry caches: a user-tool schema per exposed integration tool.
161+
let permissionConfig: IntegrationGateConfig | null = null
162+
if (workspaceId) {
163+
const { getUserPermissionConfig } = await import('@/ee/access-control/utils/permission-check')
164+
permissionConfig = await getUserPermissionConfig(userId, workspaceId)
165+
}
154166
const cacheKey = getIntegrationToolSchemaCacheKey(
155167
userId,
156168
workspaceId,
157169
schemaSurface,
158-
visibilitySignature(vis)
170+
visibilitySignature(vis),
171+
integrationGateSignature(permissionConfig)
159172
)
160173
const cached = integrationToolSchemaCache.get(cacheKey)
161174
if (cached) {
@@ -167,7 +180,8 @@ export async function buildIntegrationToolSchemas(
167180
messageId,
168181
{ schemaSurface },
169182
workspaceId,
170-
vis
183+
vis,
184+
permissionConfig
171185
).catch((error) => {
172186
integrationToolSchemaCache.delete(cacheKey)
173187
throw error
@@ -185,15 +199,11 @@ async function buildIntegrationToolSchemasUncached(
185199
messageId: string | undefined,
186200
options: Required<BuildIntegrationToolSchemasOptions>,
187201
workspaceId?: string,
188-
vis: BlockVisibilityState | null = null
202+
vis: BlockVisibilityState | null = null,
203+
permissionConfig: IntegrationGateConfig | null = null
189204
): Promise<ToolSchema[]> {
190205
const reqLogger = logger.withMetadata({ messageId })
191206
const integrationTools: ToolSchema[] = []
192-
let permissionConfig: IntegrationGateConfig | null = null
193-
if (workspaceId) {
194-
const { getUserPermissionConfig } = await import('@/ee/access-control/utils/permission-check')
195-
permissionConfig = await getUserPermissionConfig(userId, workspaceId)
196-
}
197207

198208
try {
199209
const { createUserToolSchema } = await import('@/tools/params')

apps/sim/lib/copilot/integration-tool-projection.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,22 @@ export function projectIntegrationToolsForViewer(
7474
return { tools, allowedBlockTypes, isToolAllowed }
7575
}
7676

77+
/**
78+
* Stable signature of the policy {@link projectIntegrationToolsForViewer} reads,
79+
* for keying caches whose contents depend on the projection.
80+
*
81+
* The projection is only as fresh as what keys it: an entry cached under a
82+
* viewer's identity alone outlives the policy that produced it, so an admin's
83+
* change would not take effect until the entry expired. Mirrors
84+
* `visibilitySignature`, which does the same job for block visibility.
85+
*/
86+
export function integrationGateSignature(config: IntegrationGateConfig | null | undefined): string {
87+
return JSON.stringify([
88+
config?.allowedIntegrations ? [...config.allowedIntegrations].sort() : null,
89+
config?.deniedTools?.length ? [...config.deniedTools].sort() : null,
90+
])
91+
}
92+
7793
/** What a viewer's `deniedTools` denylist costs the block schemas they are shown. */
7894
export interface DeniedBlockOperations {
7995
/**

0 commit comments

Comments
 (0)