Skip to content

Commit 8d29616

Browse files
committed
fix(selectors): gate raw-context and api-key selectors on their integration
1 parent ae8785b commit 8d29616

11 files changed

Lines changed: 252 additions & 18 deletions

File tree

apps/sim/lib/selectors/application/execute-selector.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,64 @@ describe('executeSelector', () => {
275275
expect(mocks.executeAttachment).not.toHaveBeenCalled()
276276
})
277277

278+
/**
279+
* The hole this closes: a selector authenticated from raw context fields
280+
* (CloudWatch's AWS keys, IMAP's host and password) carries no credential
281+
* policy, so the gate used to resolve it to an empty service list and return
282+
* without checking — reaching the third party with the caller's keys under an
283+
* allowlist that never named it.
284+
*/
285+
it('refuses a raw-context selector whose declared integration is excluded', async () => {
286+
mocks.getAttachment.mockReturnValue({
287+
destination: 'fixed',
288+
integrationBlockTypes: ['cloudwatch'],
289+
execute: mocks.executeAttachment,
290+
})
291+
mockResolvePermissionGroupConfig.mockResolvedValue({
292+
...DEFAULT_PERMISSION_GROUP_CONFIG,
293+
allowedIntegrations: ['slack_v2'],
294+
})
295+
296+
await expect(execute()).rejects.toBeInstanceOf(IntegrationNotAllowedError)
297+
expect(mocks.executeAttachment).not.toHaveBeenCalled()
298+
})
299+
300+
it('executes a raw-context selector whose declared integration is permitted', async () => {
301+
mocks.getAttachment.mockReturnValue({
302+
destination: 'fixed',
303+
integrationBlockTypes: ['cloudwatch'],
304+
execute: mocks.executeAttachment,
305+
})
306+
mockResolvePermissionGroupConfig.mockResolvedValue({
307+
...DEFAULT_PERMISSION_GROUP_CONFIG,
308+
allowedIntegrations: ['cloudwatch'],
309+
})
310+
311+
await expect(execute()).resolves.toMatchObject({ kind: 'list' })
312+
expect(mocks.executeAttachment).toHaveBeenCalledTimes(1)
313+
})
314+
315+
/**
316+
* An API-key integration owns no OAuth catalog entry, so its service id maps
317+
* to no block type. The declaration is what gives the allowlist something to
318+
* judge, and it must win over the catalog.
319+
*/
320+
it('refuses an api-key selector whose declared integration is excluded', async () => {
321+
mocks.getAttachment.mockReturnValue({
322+
destination: 'fixed',
323+
credential: { kind: 'stored', field: 'oauthCredential', serviceIds: ['snowflake'] },
324+
integrationBlockTypes: ['snowflake'],
325+
execute: mocks.executeAttachment,
326+
})
327+
mockResolvePermissionGroupConfig.mockResolvedValue({
328+
...DEFAULT_PERMISSION_GROUP_CONFIG,
329+
allowedIntegrations: ['slack_v2'],
330+
})
331+
332+
await expect(execute()).rejects.toBeInstanceOf(IntegrationNotAllowedError)
333+
expect(mocks.executeAttachment).not.toHaveBeenCalled()
334+
})
335+
278336
/**
279337
* A selector with no integration identity is not an integration: an internal
280338
* selector declares no credential policy at all, so an allowlist that names

apps/sim/lib/selectors/application/execute-selector.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
} from '@/lib/selectors/server/errors'
2121
import {
2222
assertSelectorIntegrationAllowed,
23-
selectorResourceServiceIds,
23+
selectorIntegrationBlockTypes,
2424
} from '@/lib/selectors/server/integration-access'
2525
import { createSelectorProtectedValues } from '@/lib/selectors/server/protected-values'
2626
import { resolveSelectorReferences } from '@/lib/selectors/server/references'
@@ -171,12 +171,15 @@ async function executeAuthorizedSelector(args: {
171171
*
172172
* Judged against the selector's own resource — the API it calls — not the
173173
* set of credentials it accepts, and not the bound credential's provider.
174-
* Placed before the provider call so a denied integration is never reached.
174+
* A selector the OAuth catalog cannot identify (raw-context credentials, an
175+
* API-key integration) declares its block types instead of resolving to
176+
* none and passing untested. Placed before the provider call so a denied
177+
* integration is never reached.
175178
*/
176179
await assertSelectorIntegrationAllowed({
177180
principal: args.principal,
178181
workspaceId: args.context.workspaceId,
179-
serviceIds: attachment.credential ? selectorResourceServiceIds(attachment.credential) : [],
182+
blockTypes: selectorIntegrationBlockTypes(attachment),
180183
})
181184

182185
const credentialAccess = credential?.access
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { selectorManifest } from '@/lib/selectors/manifest'
6+
import { selectorIntegrationBlockTypes } from '@/lib/selectors/server/integration-access'
7+
import { serverSelectorRegistry } from '@/lib/selectors/server/registry'
8+
9+
describe('selectorIntegrationBlockTypes', () => {
10+
/**
11+
* The gate passes a selector with no integration identity, so an identity it
12+
* cannot derive is a silent hole: `POST /api/selectors/execute` reaches the
13+
* third party with the caller's credentials and the group's
14+
* `allowedIntegrations` never gets a say. Every selector the manifest calls
15+
* `provider-server` must therefore resolve to at least one block type, either
16+
* through the OAuth credential catalog or by declaring one.
17+
*/
18+
it('gives every provider selector an integration identity', () => {
19+
const ungated = Object.entries(serverSelectorRegistry)
20+
.filter(([key]) => selectorManifest[key as keyof typeof selectorManifest])
21+
.filter(
22+
([key, attachment]) =>
23+
selectorManifest[key as keyof typeof selectorManifest].classification ===
24+
'provider-server' && selectorIntegrationBlockTypes(attachment).length === 0
25+
)
26+
.map(([key]) => key)
27+
28+
expect(ungated).toEqual([])
29+
})
30+
31+
/**
32+
* The other half of the same rule: an internal selector reads Sim's own
33+
* workspace data, so it is not an integration and nothing gates it.
34+
*/
35+
it('gives an internal selector no integration identity', () => {
36+
const internal = Object.entries(serverSelectorRegistry).filter(
37+
([key]) =>
38+
selectorManifest[key as keyof typeof selectorManifest]?.classification === 'internal-server'
39+
)
40+
41+
expect(internal.length).toBeGreaterThan(0)
42+
for (const [key, attachment] of internal) {
43+
expect([key, selectorIntegrationBlockTypes(attachment)]).toEqual([key, []])
44+
}
45+
})
46+
47+
/** A declaration wins over the catalog, which is what an API-key selector needs. */
48+
it('prefers a declared block type over the credential catalog', () => {
49+
expect(
50+
selectorIntegrationBlockTypes({
51+
credential: { kind: 'stored', field: 'oauthCredential', serviceIds: ['gmail'] },
52+
integrationBlockTypes: ['snowflake'],
53+
})
54+
).toEqual(['snowflake'])
55+
})
56+
57+
it('derives the block type from the credential resource when none is declared', () => {
58+
expect(
59+
selectorIntegrationBlockTypes({
60+
credential: {
61+
kind: 'stored',
62+
field: 'oauthCredential',
63+
serviceIds: ['google-drive', 'google-sheets'],
64+
resourceServiceId: 'google-drive',
65+
},
66+
})
67+
).toContain('google_drive')
68+
})
69+
})

apps/sim/lib/selectors/server/integration-access.ts

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ import {
66
isBlockTypeAccessControlExempt,
77
resolveAccessControlBlockType,
88
} from '@/lib/permission-groups/block-access'
9-
import type { SelectorCredentialPolicy } from '@/lib/selectors/server/types'
9+
import type {
10+
SelectorCredentialPolicy,
11+
ServerSelectorAttachment,
12+
} from '@/lib/selectors/server/types'
1013
import { IntegrationNotAllowedError } from '@/ee/access-control/utils/permission-check'
1114

1215
const logger = createLogger('SelectorIntegrationAccess')
@@ -32,6 +35,34 @@ export function selectorResourceServiceIds(policy: SelectorCredentialPolicy): re
3235
return policy.resourceServiceId ? [policy.resourceServiceId] : policy.serviceIds
3336
}
3437

38+
/**
39+
* The block types an allowlist decision about this selector is made against.
40+
*
41+
* Two independent sources, because the OAuth credential catalog cannot identify
42+
* every selector that reaches a third-party API. A selector authenticated from
43+
* raw context fields (CloudWatch's AWS keys, IMAP's host and password) carries
44+
* no credential policy, and an API-key integration (Snowflake, NetSuite,
45+
* Harmonic) owns no OAuth catalog entry, so its service id maps to no block
46+
* type — both used to yield an empty list and pass the gate untested. They
47+
* declare `integrationBlockTypes` instead, and it wins over the catalog when
48+
* both are present.
49+
*
50+
* An empty result means "no integration identity", which is a pass. That is
51+
* reserved for the internal selectors — workspace files, knowledge bases,
52+
* tables — which read only Sim's own data;
53+
* `selectorIntegrationCoverage` in the manifest test keeps every
54+
* `provider-server` selector out of it.
55+
*/
56+
export function selectorIntegrationBlockTypes(
57+
attachment: Pick<ServerSelectorAttachment, 'credential' | 'integrationBlockTypes'>
58+
): readonly string[] {
59+
if (attachment.integrationBlockTypes?.length) return attachment.integrationBlockTypes
60+
if (!attachment.credential) return []
61+
return selectorResourceServiceIds(attachment.credential).flatMap((serviceId) =>
62+
getIntegrationTypesForOAuthServiceId(serviceId)
63+
)
64+
}
65+
3566
/**
3667
* Refuses a selector execution whose integration the caller's permission group
3768
* does not permit.
@@ -54,13 +85,12 @@ export function selectorResourceServiceIds(policy: SelectorCredentialPolicy): re
5485
* bound to `slack_v2` match.
5586
*
5687
* A `null` allowlist, a caller no group governs, and a selector with no
57-
* integration identity all pass through. The last covers two real shapes: an
58-
* internal selector (workspace files, knowledge bases) declares no credential
59-
* policy at all, and an API-key integration — Snowflake, NetSuite, Harmonic —
60-
* owns no OAuth entry in the deployment integration catalog and therefore maps
61-
* to no block type. Treating an unmapped service as allowed is deliberate and
62-
* is what the credential catalog already does; see
63-
* `isOAuthServiceAllowedByIntegrationTypes`.
88+
* integration identity all pass through. The last is now reserved for the
89+
* internal selectors — workspace files, knowledge bases, tables — which read
90+
* only Sim's own data and are not an integration at all. Every selector that
91+
* reaches a third party has an identity, either through the OAuth credential
92+
* catalog or through the `integrationBlockTypes` a raw-context or API-key
93+
* selector declares; see {@link selectorIntegrationBlockTypes}.
6494
*
6595
* One service can still map to several block types — the `google-drive` entry
6696
* authenticates both `google_drive` and `google_slides_v2` — and any of them
@@ -71,18 +101,14 @@ export function selectorResourceServiceIds(policy: SelectorCredentialPolicy): re
71101
export async function assertSelectorIntegrationAllowed(input: {
72102
principal: Principal
73103
workspaceId: string
74-
serviceIds: readonly string[]
104+
blockTypes: readonly string[]
75105
}): Promise<void> {
76-
if (input.serviceIds.length === 0) return
106+
const blockTypes = input.blockTypes
107+
if (blockTypes.length === 0) return
77108

78109
const allowlist = await allowedIntegrationTypes(input.principal, input.workspaceId)
79110
if (allowlist === null) return
80111

81-
const blockTypes = input.serviceIds.flatMap((serviceId) =>
82-
getIntegrationTypesForOAuthServiceId(serviceId)
83-
)
84-
if (blockTypes.length === 0) return
85-
86112
const allowed = blockTypes.some(
87113
(blockType) =>
88114
isBlockTypeAccessControlExempt(blockType) ||

apps/sim/lib/selectors/server/providers/cloudwatch.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,16 @@ async function executeCloudWatchListing<T>(
5353
}
5454
}
5555

56+
/**
57+
* The integration this selector reaches. Declared rather than derived: the selector authenticates from raw AWS keys in the request context and
58+
* carries no stored connection, so the OAuth credential catalog can identify
59+
* nothing to gate it on.
60+
*/
61+
const integrationBlockTypes = ['cloudwatch'] as const
62+
5663
export const cloudWatchSelectorAttachments = {
5764
'cloudwatch.logGroups': {
65+
integrationBlockTypes,
5866
destination: 'fixed',
5967
async execute(args) {
6068
const listingCredentials = credentials(args.context)
@@ -94,6 +102,7 @@ export const cloudWatchSelectorAttachments = {
94102
},
95103
},
96104
'cloudwatch.logStreams': {
105+
integrationBlockTypes,
97106
destination: 'fixed',
98107
async execute(args) {
99108
const listingCredentials = credentials(args.context)

apps/sim/lib/selectors/server/providers/harmonic.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,16 @@ async function executeSavedSearches(args: ExecuteServerSelectorArgs) {
175175
)
176176
}
177177

178+
/**
179+
* The integration this selector reaches. Declared rather than derived: Harmonic is an API-key integration with no entry in the deployment OAuth
180+
* catalog, so its service id maps to no block type.
181+
*/
182+
const integrationBlockTypes = ['harmonic'] as const
183+
178184
export const harmonicSelectorAttachments = {
179185
'harmonic.savedSearches': {
180186
credential: { kind: 'stored', field: 'oauthCredential', serviceIds: ['harmonic'] },
187+
integrationBlockTypes,
181188
destination: 'fixed',
182189
execute: executeSavedSearches,
183190
},

apps/sim/lib/selectors/server/providers/imap.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,16 @@ function throwPublicImapError(error: unknown): never {
1919
throw new SelectorConnectionUnavailableError()
2020
}
2121

22+
/**
23+
* The integration this selector reaches. Declared rather than derived: the selector opens an IMAP connection from raw host and password fields in
24+
* the request context and carries no stored connection, so the OAuth
25+
* credential catalog can identify nothing to gate it on.
26+
*/
27+
const integrationBlockTypes = ['imap'] as const
28+
2229
export const imapSelectorAttachments = {
2330
'imap.mailboxes': definePreparedSelectorAttachment({
31+
integrationBlockTypes,
2432
destination: {
2533
kind: 'user-controlled',
2634
async prepare(args) {

apps/sim/lib/selectors/server/providers/managed-agent.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,27 +97,39 @@ const credential = {
9797
serviceIds: ['claude-platform'],
9898
} as const
9999

100+
/**
101+
* The integration this selector reaches. Declared rather than derived: The managed-agent platform is an
102+
* API-key integration with no entry in the deployment OAuth catalog, so its
103+
* service id maps to no block type and the allowlist would have nothing to
104+
* judge it on.
105+
*/
106+
const integrationBlockTypes = ['managed_agent'] as const
107+
100108
export const managedAgentSelectorAttachments = {
101109
'managedAgent.agents': {
102110
credential,
111+
integrationBlockTypes,
103112
destination: 'fixed',
104113
auditCredentialUse: true,
105114
execute: (args) => executeResource(args, 'agents'),
106115
},
107116
'managedAgent.environments': {
108117
credential,
118+
integrationBlockTypes,
109119
destination: 'fixed',
110120
auditCredentialUse: true,
111121
execute: (args) => executeResource(args, 'environments'),
112122
},
113123
'managedAgent.vaults': {
114124
credential,
125+
integrationBlockTypes,
115126
destination: 'fixed',
116127
auditCredentialUse: true,
117128
execute: (args) => executeResource(args, 'vaults'),
118129
},
119130
'managedAgent.memoryStores': {
120131
credential,
132+
integrationBlockTypes,
121133
destination: 'fixed',
122134
auditCredentialUse: true,
123135
execute: (args) => executeResource(args, 'memory-stores'),

apps/sim/lib/selectors/server/providers/netsuite.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,14 +238,24 @@ const credential = {
238238
serviceIds: ['netsuite'],
239239
} as const
240240

241+
/**
242+
* The integration this selector reaches. Declared rather than derived: NetSuite is an
243+
* API-key integration with no entry in the deployment OAuth catalog, so its
244+
* service id maps to no block type and the allowlist would have nothing to
245+
* judge it on.
246+
*/
247+
const integrationBlockTypes = ['netsuite'] as const
248+
241249
export const netsuiteSelectorAttachments = {
242250
'netsuite.recordTypes': definePreparedSelectorAttachment({
243251
credential,
252+
integrationBlockTypes,
244253
destination: { kind: 'credential-bound', prepare: prepareNetSuiteDestination },
245254
execute: executeNetSuite,
246255
}),
247256
'netsuite.asyncTasks': definePreparedSelectorAttachment({
248257
credential,
258+
integrationBlockTypes,
249259
destination: { kind: 'credential-bound', prepare: prepareNetSuiteDestination },
250260
execute: executeNetSuite,
251261
}),

0 commit comments

Comments
 (0)