Skip to content

Commit 1a387ed

Browse files
committed
fix(permission-groups): gate selector execution on allowedIntegrations, apply the personal-key policy on v2 chat
Three cubic findings on the enforcement PR. `POST /api/selectors/execute` reaches a provider's API with the caller's credential, so it is a use of the integration and not a neutral picker. The authorization funnel cannot apply `allowedIntegrations` because it never sees which integration a selector key stands for, so the decision is asserted from the use case, after credential binding and ahead of the provider call. The integration identity comes from the selector attachment's declared OAuth services, narrowed by the resolved credential's provider id so a two-service selector is judged as the half the caller actually reaches. `/api/v2/chat` only ever runs for a personal API key, and `admitV2Request` authenticates without authorizing, so neither half of the funnel's personal-key policy applied there. Both now run after the workspace access check, the group half through the shared `requirePersonalApiKeysAllowed`. The raw copilot chat route's `copilot.use` refusal now renders through `capabilityRefusalResponse`, so it carries the same detail code as every other capability refusal.
1 parent bdeb07a commit 1a387ed

10 files changed

Lines changed: 425 additions & 17 deletions

File tree

apps/sim/app/api/selectors/execute/route.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ import {
7777
SelectorOptionsUnavailableError,
7878
} from '@/lib/selectors/server/errors'
7979
import { POST } from '@/app/api/selectors/execute/route'
80+
import { IntegrationNotAllowedError } from '@/ee/access-control/utils/permission-check'
8081

8182
function project(error: unknown) {
8283
const result = mocks.errorPolicy?.project(error)
@@ -126,6 +127,22 @@ describe('POST /api/selectors/execute', () => {
126127
})
127128
})
128129

130+
/**
131+
* The one selector failure that names itself. The other three are normalized
132+
* so a caller cannot probe a scope or a credential through them; this one
133+
* reports the caller's own permission group against their own workspace and
134+
* names the remedy, which "Connection unavailable" would hide.
135+
*/
136+
it('projects an integration-allowlist refusal as its own 403', () => {
137+
expect(project(new IntegrationNotAllowedError('gmail_v2'))).toEqual({
138+
status: 403,
139+
body: {
140+
error: 'Integration "gmail_v2" is not allowed based on your permission group settings',
141+
},
142+
headers: { 'Cache-Control': 'private, no-store' },
143+
})
144+
})
145+
129146
it('preserves same-workspace forbidden errors', () => {
130147
expect(
131148
project(new OrchestrationError('forbidden', 'Insufficient workspace permissions'))

apps/sim/app/api/selectors/execute/route.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
SelectorContextUnavailableError,
1818
SelectorOptionsUnavailableError,
1919
} from '@/lib/selectors/server/errors'
20+
import { IntegrationNotAllowedError } from '@/ee/access-control/utils/permission-check'
2021

2122
const PRIVATE_NO_STORE = { 'Cache-Control': 'private, no-store' } as const
2223
const SELECTOR_SCOPE_NOT_FOUND = 'Selector scope not found'
@@ -34,6 +35,17 @@ const selectorOperationErrorPolicy = extendInternalErrorPolicy(
3435
PRIVATE_NO_STORE
3536
)
3637
}
38+
/**
39+
* The integration allowlist refusal, which is deliberately the one selector
40+
* failure that names itself. The other three are normalized so a caller
41+
* cannot probe a scope or a credential through them; this one reports the
42+
* caller's OWN permission group against their own workspace, tells them the
43+
* remedy is an admin changing the allowlist rather than a broken connection,
44+
* and reveals nothing they could not read off the block toolbar.
45+
*/
46+
if (error instanceof IntegrationNotAllowedError) {
47+
return internalErrorResponse(403, { error: error.message }, PRIVATE_NO_STORE)
48+
}
3749
if (error instanceof SelectorOptionsUnavailableError) {
3850
return internalErrorResponse(
3951
error.status,

apps/sim/app/api/v2/chat/route.test.ts

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,10 @@ describe('POST /api/v2/chat', () => {
252252
mockAuthenticateV2ApiKey.mockResolvedValue(personalAuth)
253253
mockCheckPreAuthRate.mockResolvedValue({ allowed: true, remaining: 10, resetAt: new Date() })
254254
mockCheckOperationRate.mockResolvedValue({ allowed: true, remaining: 10, resetAt: new Date() })
255-
mockAssertActiveWorkspaceAccess.mockResolvedValue({ permission: 'admin' })
255+
mockAssertActiveWorkspaceAccess.mockResolvedValue({
256+
permission: 'admin',
257+
workspace: { organizationId: null, allowPersonalApiKeys: true },
258+
})
256259
mockResolvePermissionGroupConfig.mockResolvedValue(null)
257260
mockResolveBillingAttribution.mockResolvedValue(billingAttributionSnapshot)
258261
mockRequestExplicitStreamAbort.mockResolvedValue(undefined)
@@ -331,6 +334,77 @@ describe('POST /api/v2/chat', () => {
331334
expect(mockRunHeadlessCopilotLifecycle).not.toHaveBeenCalled()
332335
})
333336

337+
/**
338+
* The route only ever runs for a personal API key, and `admitV2Request` never
339+
* authorizes, so both halves of the funnel's personal-key policy have to be
340+
* repeated here. The workspace column is the first half.
341+
*/
342+
it('answers 403 when the workspace has switched personal API keys off', async () => {
343+
mockAssertActiveWorkspaceAccess.mockResolvedValue({
344+
permission: 'admin',
345+
workspace: { organizationId: null, allowPersonalApiKeys: false },
346+
})
347+
348+
const response = await callChat({ workspaceId: 'workspace-1', message: 'hi' })
349+
350+
expect(response.status).toBe(403)
351+
await expect(response.json()).resolves.toEqual({
352+
error: {
353+
code: 'FORBIDDEN',
354+
message: 'Personal API keys are not allowed for this workspace',
355+
details: { code: 'PERSONAL_API_KEYS_DISABLED' },
356+
},
357+
})
358+
expect(mockResolveOrCreateChat).not.toHaveBeenCalled()
359+
expect(mockRunHeadlessCopilotLifecycle).not.toHaveBeenCalled()
360+
})
361+
362+
/**
363+
* The group half. The column and the key combine with AND, so a workspace
364+
* that allows personal keys still refuses the cohort whose group withholds
365+
* them — the case `copilot.use` alone could never see.
366+
*/
367+
it('answers 403 when the permission group withholds personal_api_key.use', async () => {
368+
mockAssertActiveWorkspaceAccess.mockResolvedValue({
369+
permission: 'admin',
370+
workspace: { organizationId: 'org-1', allowPersonalApiKeys: true },
371+
})
372+
mockResolvePermissionGroupConfig.mockResolvedValue({
373+
...DEFAULT_PERMISSION_GROUP_CONFIG,
374+
disablePersonalApiKeys: true,
375+
})
376+
377+
const response = await callChat({ workspaceId: 'workspace-1', message: 'hi' })
378+
379+
expect(response.status).toBe(403)
380+
await expect(response.json()).resolves.toEqual({
381+
error: {
382+
code: 'FORBIDDEN',
383+
message: 'Personal API keys are not allowed for this workspace',
384+
details: { code: 'PERSONAL_API_KEYS_DISABLED' },
385+
},
386+
})
387+
expect(mockResolveOrCreateChat).not.toHaveBeenCalled()
388+
expect(mockRunHeadlessCopilotLifecycle).not.toHaveBeenCalled()
389+
})
390+
391+
/** A workspace with no organization resolves no group, so the key passes. */
392+
it('runs one turn for a personal key in a workspace no group governs', async () => {
393+
mockAssertActiveWorkspaceAccess.mockResolvedValue({
394+
permission: 'admin',
395+
workspace: { organizationId: null, allowPersonalApiKeys: true },
396+
})
397+
mockResolvePermissionGroupConfig.mockResolvedValue({
398+
...DEFAULT_PERMISSION_GROUP_CONFIG,
399+
disablePersonalApiKeys: true,
400+
})
401+
402+
const response = await callChat({ workspaceId: 'workspace-1', message: 'hi' })
403+
404+
expect(response.status).toBe(200)
405+
expect(mockRunHeadlessCopilotLifecycle).toHaveBeenCalledTimes(1)
406+
})
407+
334408
/** Workspace reach is decided first, so the refusal cannot name a group to an outsider. */
335409
it('refuses an inaccessible workspace before consulting a permission group', async () => {
336410
mockAssertActiveWorkspaceAccess.mockRejectedValue(new MockWorkspaceAccessDeniedError('denied'))

apps/sim/app/api/v2/chat/route.ts

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { createLogger } from '@sim/logger'
22
import { getErrorMessage, toError } from '@sim/utils/errors'
33
import { generateId } from '@sim/utils/id'
44
import { truncate } from '@sim/utils/string'
5-
import type { NextRequest } from 'next/server'
5+
import type { NextRequest, NextResponse } from 'next/server'
66
import { v2ChatContract } from '@/lib/api/contracts/v2/chat'
77
import { parseRequest } from '@/lib/api/server'
88
import {
@@ -36,6 +36,12 @@ import { runHeadlessCopilotLifecycle } from '@/lib/copilot/request/lifecycle/hea
3636
import { requestExplicitStreamAbort } from '@/lib/copilot/request/session/explicit-abort'
3737
import type { OrchestratorResult, StreamEvent } from '@/lib/copilot/request/types'
3838
import { normalizeSecretMountPolicy } from '@/lib/copilot/secret-mount-policy'
39+
import {
40+
forbiddenErrorDetails,
41+
PersonalApiKeysDisabledError,
42+
requirePersonalApiKeysAllowed,
43+
type WorkspaceAuthorizationContext,
44+
} from '@/lib/core/application'
3945
import { isDocSandboxEnabled } from '@/lib/core/config/env-flags'
4046
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
4147
import { getPersonalAndWorkspaceEnv } from '@/lib/environment/utils'
@@ -79,6 +85,34 @@ function deriveConversationTitle(message: string): string | undefined {
7985
return truncate(normalized, CHAT_TITLE_MAX_LENGTH)
8086
}
8187

88+
/**
89+
* The two personal-API-key checks `authorizeWorkspaceOperation` applies, for the
90+
* one route that never reaches it, or `null` when the key may proceed.
91+
*
92+
* The group half runs through the same {@link requirePersonalApiKeysAllowed} the
93+
* funnel and the billing reads call, so a third wording of the same refusal
94+
* cannot drift in. Its error is projected rather than thrown because this route
95+
* renders its own v2 envelope, and the detail code is read off the error so the
96+
* column refusal and the group refusal answer with one code.
97+
*/
98+
async function personalApiKeyPolicyRefusal(
99+
userId: string,
100+
context: WorkspaceAuthorizationContext
101+
): Promise<NextResponse | null> {
102+
const refuse = (error: PersonalApiKeysDisabledError) =>
103+
v2Error('FORBIDDEN', error.message, { details: forbiddenErrorDetails(error) })
104+
105+
if (!context.allowPersonalApiKeys) return refuse(new PersonalApiKeysDisabledError())
106+
107+
try {
108+
await requirePersonalApiKeysAllowed(userId, context)
109+
} catch (error) {
110+
if (error instanceof PersonalApiKeysDisabledError) return refuse(error)
111+
throw error
112+
}
113+
return null
114+
}
115+
82116
function isAbortError(error: unknown): boolean {
83117
return error instanceof Error && error.name === 'AbortError'
84118
}
@@ -169,6 +203,31 @@ export const POST = withRouteHandler(
169203
const workspaceAccess = await assertActiveWorkspaceAccess(workspaceId, userId)
170204
const userPermission = workspaceAccess.permission
171205

206+
/**
207+
* permission-group-enforced: personal_api_key.use — this route only ever
208+
* runs for a personal API key, and `admitV2Request` authenticates one
209+
* without authorizing it, so the funnel's personal-key policy has to be
210+
* repeated here or the same key `authorizeWorkspaceOperation` refuses
211+
* still starts a chat turn.
212+
*
213+
* Both halves, because they combine with AND: the workspace column is the
214+
* coarse switch every workspace has, and the group key narrows it further
215+
* for one cohort inside an enterprise organization. Either one saying no
216+
* is a no, and checking only `copilot.use` applied neither.
217+
*
218+
* Both run after workspace access rather than before it, unlike the
219+
* funnel, which can afford to check the column first because its caller
220+
* has already loaded the workspace. Here the access check is what loads
221+
* it, and answering later only ever conceals more: a caller with no reach
222+
* into the workspace is refused without learning how it is configured.
223+
*/
224+
const personalKeyRefusal = await personalApiKeyPolicyRefusal(userId, {
225+
workspaceId,
226+
workspaceOrganizationId: workspaceAccess.workspace?.organizationId ?? null,
227+
allowPersonalApiKeys: workspaceAccess.workspace?.allowPersonalApiKeys ?? false,
228+
})
229+
if (personalKeyRefusal) return personalKeyRefusal
230+
172231
/**
173232
* permission-group-enforced: copilot.use — read off the operation so this
174233
* route and the funnel can never name different capabilities.

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -911,6 +911,15 @@ describe('handleUnifiedChatPost', () => {
911911

912912
describe('handleUnifiedChatPost copilot.use capability gate', () => {
913913
const REFUSAL = "Chat is not available under your organization's permission group"
914+
/**
915+
* The body every raw capability refusal renders, detail code included. This
916+
* route builds it through the shared `capabilityRefusalResponse`, so a client
917+
* cannot tell a group refusal here apart from one raised by the funnel.
918+
*/
919+
const REFUSAL_BODY = {
920+
error: REFUSAL,
921+
details: { code: 'PERMISSION_GROUP_CAPABILITY_BLOCKED' },
922+
}
914923

915924
function chatRequest(body: Record<string, unknown> = {}) {
916925
return new NextRequest('http://localhost/api/copilot/chat', {
@@ -978,7 +987,7 @@ describe('handleUnifiedChatPost copilot.use capability gate', () => {
978987
const response = await handleUnifiedChatPost(chatRequest({ createNewChat: true }))
979988

980989
expect(response.status).toBe(403)
981-
await expect(response.json()).resolves.toEqual({ error: REFUSAL })
990+
await expect(response.json()).resolves.toEqual(REFUSAL_BODY)
982991
expect(resolveOrCreateChat).not.toHaveBeenCalled()
983992
expect(createSSEStream).not.toHaveBeenCalled()
984993
expect(releaseChatSendClaim).toHaveBeenCalledTimes(1)
@@ -1003,7 +1012,7 @@ describe('handleUnifiedChatPost copilot.use capability gate', () => {
10031012
)
10041013

10051014
expect(response.status).toBe(403)
1006-
await expect(response.json()).resolves.toEqual({ error: REFUSAL })
1015+
await expect(response.json()).resolves.toEqual(REFUSAL_BODY)
10071016
expect(resolvePermissionGroupConfig).toHaveBeenCalledWith('user-1', 'ws-1', undefined)
10081017
expect(createSSEStream).not.toHaveBeenCalled()
10091018
})
@@ -1021,7 +1030,7 @@ describe('handleUnifiedChatPost copilot.use capability gate', () => {
10211030
)
10221031

10231032
expect(response.status).toBe(403)
1024-
await expect(response.json()).resolves.toEqual({ error: REFUSAL })
1033+
await expect(response.json()).resolves.toEqual(REFUSAL_BODY)
10251034
expect(resolvePermissionGroupConfig).not.toHaveBeenCalledWith(
10261035
'user-1',
10271036
'ws-unrestricted',

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

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,7 @@ import {
4646
import { TraceAttr } from '@/lib/copilot/generated/trace-attributes-v1'
4747
import { TraceSpan } from '@/lib/copilot/generated/trace-spans-v1'
4848
import type { VfsSnapshotV1 } from '@/lib/copilot/generated/vfs-snapshot-v1'
49-
import {
50-
createBadRequestResponse,
51-
createForbiddenResponse,
52-
createUnauthorizedResponse,
53-
} from '@/lib/copilot/request/http'
49+
import { createBadRequestResponse, createUnauthorizedResponse } from '@/lib/copilot/request/http'
5450
import { createSSEStream, SSE_RESPONSE_HEADERS } from '@/lib/copilot/request/lifecycle/start'
5551
import { startCopilotOtelRoot, withCopilotSpan } from '@/lib/copilot/request/otel'
5652
import {
@@ -68,10 +64,8 @@ import {
6864
import { prepareExecutionContext } from '@/lib/copilot/tools/handlers/context'
6965
import type { AtomicClaimResult } from '@/lib/core/idempotency'
7066
import { chatSendIdempotency } from '@/lib/core/idempotency'
71-
import {
72-
capabilityRefusal,
73-
isWorkspaceCapabilityWithheld,
74-
} from '@/lib/permission-groups/capability-assertions'
67+
import { isWorkspaceCapabilityWithheld } from '@/lib/permission-groups/capability-assertions'
68+
import { capabilityRefusalResponse } from '@/lib/permission-groups/capability-response'
7569
import { captureServerEvent } from '@/lib/posthog/server'
7670
import { resolveWorkflowIdForUser } from '@/lib/workflows/utils'
7771
import {
@@ -1156,7 +1150,7 @@ export async function handleUnifiedChatPost(req: NextRequest) {
11561150
) {
11571151
activeOtelRoot.span.setAttribute(TraceAttr.HttpStatusCode, 403)
11581152
activeOtelRoot.finish('error')
1159-
return createForbiddenResponse(capabilityRefusal('copilot.use'))
1153+
return capabilityRefusalResponse('copilot.use')
11601154
}
11611155

11621156
let currentChat: ChatLoadResult['chat'] = null

0 commit comments

Comments
 (0)