Skip to content

Commit 6e1b053

Browse files
committed
refactor(copilot): read the chat capability off its operation declaration
The raw chat handler restated copilot.use in both the assertion and the refusal, so changing chatOperations.send's policy would leave the route over- or under-gated and answering the wrong refusal code. Read the declaration once instead, including its 'none' case, where a declarative surface asserts nothing and so does this.
1 parent ad317b5 commit 6e1b053

2 files changed

Lines changed: 19 additions & 2 deletions

File tree

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ vi.mock('@/lib/copilot/chat-status', () => ({
141141
},
142142
}))
143143

144+
import { chatOperations } from '@/lib/copilot/application/operations'
144145
import { DEFAULT_PERMISSION_GROUP_CONFIG } from '@/lib/permission-groups/fields'
145146
import { handleUnifiedChatPost } from './post'
146147

@@ -978,6 +979,15 @@ describe('handleUnifiedChatPost copilot.use capability gate', () => {
978979
* taken first and released by the handler's `finally`, so a retry is free to
979980
* start a turn.
980981
*/
982+
/**
983+
* The capability this raw handler asserts is the one `chatOperations.send`
984+
* declares, not a literal restated beside it — a declarative surface would
985+
* enforce the declaration, and this one must agree with it.
986+
*/
987+
it('enforces the capability the chat operation declares', () => {
988+
expect(chatOperations.send.capability).toBe('copilot.use')
989+
})
990+
981991
it('refuses the send when the group withholds copilot.use', async () => {
982992
resolvePermissionGroupConfig.mockResolvedValue({
983993
...DEFAULT_PERMISSION_GROUP_CONFIG,

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { z } from 'zod'
1010
import { isZodError, validationErrorResponse } from '@/lib/api/server'
1111
import { getSession } from '@/lib/auth'
1212
import { resolveBillingAttribution } from '@/lib/billing/core/billing-attribution'
13+
import { chatOperations } from '@/lib/copilot/application/operations'
1314
import {
1415
DESKTOP_TERMINAL_HINT_ID_MAX_LENGTH,
1516
DESKTOP_TERMINAL_HINT_TEXT_MAX_LENGTH,
@@ -1125,6 +1126,10 @@ export async function handleUnifiedChatPost(req: NextRequest) {
11251126
/**
11261127
* permission-group-enforced: copilot.use — Chat is a raw handler rather
11271128
* than a workspace operation, so the authorization funnel never sees it.
1129+
* The capability is read off `chatOperations.send` rather than restated,
1130+
* so the assertion and the refusal cannot drift from the declaration a
1131+
* declarative surface would enforce — including the `'none'` case, where
1132+
* a declarative surface asserts nothing and so does this.
11281133
*
11291134
* Gated on the workspace the turn actually lands in, which is the one
11301135
* `resolveBranch` just resolved rather than the one the request asked
@@ -1140,17 +1145,19 @@ export async function handleUnifiedChatPost(req: NextRequest) {
11401145
* taken above is released by the `finally`, so a refused send leaves a
11411146
* later retry free to start a turn.
11421147
*/
1148+
const chatCapability = chatOperations.send.capability
11431149
if (
11441150
branch.workspaceId &&
1151+
chatCapability !== 'none' &&
11451152
(await isWorkspaceCapabilityWithheld(
11461153
authenticatedUserId,
11471154
branch.workspaceId,
1148-
'copilot.use'
1155+
chatCapability
11491156
))
11501157
) {
11511158
activeOtelRoot.span.setAttribute(TraceAttr.HttpStatusCode, 403)
11521159
activeOtelRoot.finish('error')
1153-
return capabilityRefusalResponse('copilot.use')
1160+
return capabilityRefusalResponse(chatCapability)
11541161
}
11551162

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

0 commit comments

Comments
 (0)