Skip to content

Commit 6a03859

Browse files
committed
fix(access-control): close two denied-tool leaks in copilot discovery
The VFS stamped every integration schema from the shared static map before the per-viewer loop re-authored the permitted subset, so a denied operation's schema stayed published. Skip the shared copy for integration paths; the viewer loop is the only projection that knows the denylist. Block metadata resolved denied operations from the catalog's `operation.toolId`, which the projection fills only from `tools.config.tool` — a block whose operation ids are its tool ids left it undefined and read as fully permitted. Resolve through the shared operation gate instead.
1 parent 0d34302 commit 6a03859

4 files changed

Lines changed: 90 additions & 5 deletions

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { getExposedIntegrationTools } from '@/lib/copilot/integration-tools'
6+
import { BLOCK_REGISTRY } from '@/blocks/registry-maps'
7+
8+
/**
9+
* Sweeps the real registry for the invariant the permission gate depends on.
10+
*
11+
* A permission group's `deniedTools` holds the ids an admin sees in the access
12+
* control grid, which are exactly the owning block's `tools.access` entries.
13+
* The gate compares ids verbatim, so if an exposed tool were ever published
14+
* under an id its block does not declare — a `_v2` superseding a still-declared
15+
* v1, say — denying the declared id would leave the exposed one advertised and
16+
* callable. Pin it here so that authoring mistake fails at CI rather than
17+
* silently widening what a governed workspace can reach.
18+
*/
19+
describe('exposed integration tool invariants', () => {
20+
it('publishes every tool under an id its owning block declares', () => {
21+
const drift = getExposedIntegrationTools()
22+
.filter(
23+
(tool) => !(BLOCK_REGISTRY[tool.blockType]?.tools?.access ?? []).includes(tool.toolId)
24+
)
25+
.map((tool) => `${tool.blockType} publishes ${tool.toolId}, which it does not declare`)
26+
27+
expect(drift).toEqual([])
28+
})
29+
})

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,50 @@ describe('get blocks metadata', () => {
153153
expect(Object.keys(slack.operations).sort()).toEqual(['canvas', 'send'])
154154
})
155155

156+
/**
157+
* A block whose operation ids ARE its tool ids, declaring no
158+
* `tools.config.tool`. The catalog projection cannot fill `operation.toolId`
159+
* for it, so gating on that field alone would publish every denied operation.
160+
*/
161+
const selectorlessBlock = {
162+
type: 'sqs',
163+
name: 'SQS',
164+
description: 'Queue.',
165+
category: 'tools',
166+
bgColor: '#000000',
167+
icon: () => null,
168+
subBlocks: [
169+
{
170+
id: 'operation',
171+
title: 'Operation',
172+
type: 'dropdown',
173+
options: [
174+
{ label: 'Send', id: 'sqs_send' },
175+
{ label: 'Receive', id: 'sqs_receive' },
176+
],
177+
},
178+
],
179+
tools: { access: ['sqs_send', 'sqs_receive'] },
180+
inputs: {},
181+
outputs: {},
182+
} as unknown as BlockConfig
183+
184+
it('withholds a denied operation on a block that declares no tool selector', async () => {
185+
mockGetUserPermissionConfig.mockResolvedValue({
186+
allowedIntegrations: ['sqs'],
187+
deniedTools: ['sqs_receive'],
188+
})
189+
vi.mocked(getBlock).mockReturnValue(selectorlessBlock)
190+
191+
const result = await getBlocksMetadataServerTool.execute(
192+
{ blockIds: ['sqs'] },
193+
{ userId: 'user-1', workspaceId: 'workspace-1' }
194+
)
195+
196+
const sqs = result.metadata.sqs as { operations: Record<string, unknown> }
197+
expect(Object.keys(sqs.operations)).toEqual(['sqs_send'])
198+
})
199+
156200
it('withholds a block whose every operation the group denies', async () => {
157201
mockGetUserPermissionConfig.mockResolvedValue({
158202
allowedIntegrations: ['slack'],

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@ import { getServiceAccountProviderForProviderId } from '@/lib/oauth/utils'
2020
import { isBlockTypeAccessControlExempt } from '@/lib/permission-groups/block-access'
2121
import { intersectIntegrationAllowlists } from '@/lib/permission-groups/integration-allowlist'
2222
import {
23+
collectDeniedOperationIds,
2324
createToolAccessGate,
2425
type IsToolAllowed,
2526
OPERATION_SUBBLOCK_ID,
27+
type OperationGateBlock,
2628
} from '@/lib/permission-groups/operation-access'
2729
import { getBlock } from '@/blocks/registry'
2830
import { AuthMode, type BlockConfig, type SubBlockConfig } from '@/blocks/types'
@@ -134,13 +136,15 @@ function toCopilotBlockMetadata(detail: CatalogBlockDetail): CopilotBlockMetadat
134136
*/
135137
function withDeniedToolsRemoved(
136138
metadata: CopilotBlockMetadata,
139+
block: OperationGateBlock,
137140
isToolAllowed: IsToolAllowed
138141
): CopilotBlockMetadata | null {
139142
const operations = metadata.operations ?? {}
140-
const deniedOperations = new Set<string>()
141-
for (const [operationId, operation] of Object.entries(operations)) {
142-
if (operation.toolId && !isToolAllowed(operation.toolId)) deniedOperations.add(operationId)
143-
}
143+
/* Resolved through the shared operation gate rather than `operation.toolId`:
144+
the catalog projection fills that field only from `tools.config.tool`, so a
145+
block whose operation ids ARE its tool ids leaves it undefined and every one
146+
of its operations would read as permitted. */
147+
const deniedOperations = collectDeniedOperationIds(block, Object.keys(operations), isToolAllowed)
144148
const tools = metadata.tools.filter((tool) => isToolAllowed(tool.id))
145149
if (deniedOperations.size === 0 && tools.length === metadata.tools.length) return metadata
146150

@@ -278,7 +282,7 @@ export const getBlocksMetadataServerTool: BaseServerTool<
278282
continue
279283
}
280284

281-
const permitted = withDeniedToolsRemoved(metadata, isToolAllowed)
285+
const permitted = withDeniedToolsRemoved(metadata, blockConfig, isToolAllowed)
282286
if (!permitted) {
283287
logger.debug('Block has no operation this permission group allows', { blockId })
284288
continue

apps/sim/lib/copilot/vfs/workspace-vfs.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,7 @@ function getStaticToolConfigs(): ReadonlyMap<string, ToolConfig> {
455455
}
456456

457457
const BLOCK_SCHEMA_PATH_PREFIX = 'components/blocks/'
458+
const INTEGRATION_SCHEMA_PATH_PREFIX = 'components/integrations/'
458459

459460
/** The per-viewer projections applied to a shared static component file. */
460461
interface StaticFileProjection {
@@ -1049,6 +1050,13 @@ export class WorkspaceVFS {
10491050
isToolAllowed,
10501051
}
10511052
for (const [path, content] of getStaticComponentFiles()) {
1053+
/* Integration schemas are authored per viewer from
1054+
`viewerIntegrationTools` immediately below, which is the only
1055+
projection that knows the group's per-tool denylist. Stamping
1056+
the shared copy first would publish a denied operation's schema
1057+
that the loop below never overwrites, because it only writes the
1058+
operations the viewer may use. */
1059+
if (path.startsWith(INTEGRATION_SCHEMA_PATH_PREFIX)) continue
10521060
if (isStaticFileHidden(path, blockVisibility, staticFileGate)) continue
10531061
this.files.set(path, projectStaticComponentFile(path, content, staticFileProjection))
10541062
}

0 commit comments

Comments
 (0)