Skip to content

Commit 9b21b03

Browse files
committed
adjust perms
1 parent f61bd37 commit 9b21b03

10 files changed

Lines changed: 67 additions & 108 deletions

File tree

apps/sim/lib/copilot/request/tools/executor.test.ts

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ describe('pendingToolWaitBudgetMs', () => {
3636
})
3737

3838
describe('buildToolExecutionContext', () => {
39-
it('threads logical tool-call identity and server approval into the handler context', () => {
39+
it('threads logical tool-call identity into the handler context', () => {
4040
const executionContext: ExecutionContext = {
4141
userId: 'user-1',
4242
workflowId: 'workflow-1',
@@ -48,28 +48,13 @@ describe('buildToolExecutionContext', () => {
4848
{
4949
id: 'call-1',
5050
parentToolCallId: 'parent-1',
51-
userApproved: true,
5251
},
5352
executionContext
5453
)
5554
).toMatchObject({
5655
runId: 'run-1',
5756
toolCallId: 'call-1',
5857
parentToolCallId: 'parent-1',
59-
userApprovedToolCall: true,
60-
})
61-
})
62-
63-
it('does not inherit approval from the turn-scoped context', () => {
64-
const executionContext: ExecutionContext = {
65-
userId: 'user-1',
66-
workflowId: 'workflow-1',
67-
userApprovedToolCall: true,
68-
}
69-
70-
expect(buildToolExecutionContext({ id: 'call-2' }, executionContext)).toMatchObject({
71-
toolCallId: 'call-2',
72-
userApprovedToolCall: false,
7358
})
7459
})
7560
})

apps/sim/lib/copilot/request/tools/executor.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,13 +257,12 @@ class ToolExecutionTimeoutError extends Error {
257257

258258
/** Builds the per-call context from the turn-scoped execution context. */
259259
export function buildToolExecutionContext(
260-
toolCall: Pick<ToolCallState, 'id' | 'parentToolCallId' | 'userApproved'>,
260+
toolCall: Pick<ToolCallState, 'id' | 'parentToolCallId'>,
261261
execContext: ExecutionContext
262262
): ExecutionContext {
263263
return {
264264
...execContext,
265265
toolCallId: toolCall.id,
266-
userApprovedToolCall: toolCall.userApproved === true,
267266
...(toolCall.parentToolCallId ? { parentToolCallId: toolCall.parentToolCallId } : {}),
268267
}
269268
}

apps/sim/lib/copilot/request/tools/permission.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,14 @@ describe('toolCallNeedsApproval', () => {
9595
})
9696

9797
it.each(['deploy_api', 'deploy_chat', 'deploy_mcp'])(
98-
'always gates a %s undeploy even when the tool was previously allowed',
98+
'honors the saved permission for a %s undeploy',
9999
(toolName) => {
100100
const context = makeContext()
101101
context.toolPermissions.autoAllowed.add(toolName)
102102

103-
expect(toolCallNeedsApproval(toolName, context, {}, false, { action: 'undeploy' })).toBe(true)
104-
expect(toolCallNeedsApproval(toolName, context, {}, false, { action: 'deploy' })).toBe(false)
103+
expect(toolCallNeedsApproval(toolName, context, {}, false, { action: 'undeploy' })).toBe(
104+
false
105+
)
105106
}
106107
)
107108

@@ -316,7 +317,6 @@ describe('runGatedToolExecution', () => {
316317
await gate(context, toolCall, execute, [])
317318

318319
expect(execute).toHaveBeenCalledTimes(1)
319-
expect(toolCall.userApproved).toBe(true)
320320
expect(context.toolPermissions.autoAllowed.has('terminal')).toBe(true)
321321
})
322322

apps/sim/lib/copilot/request/tools/permission.ts

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,6 @@ function terminalOperationNeedsApproval(args: Record<string, unknown> | undefine
4343
return args?.operation === 'run'
4444
}
4545

46-
/** Destructive deployment calls always require approval for the exact call. */
47-
function callRequiresFreshApproval(
48-
toolName: string,
49-
args: Record<string, unknown> | undefined
50-
): boolean {
51-
return (
52-
(toolName === 'deploy_api' || toolName === 'deploy_chat' || toolName === 'deploy_mcp') &&
53-
args?.action === 'undeploy'
54-
)
55-
}
56-
5746
/**
5847
* A human can take as long as they like to answer, so the wait is bounded only
5948
* by the overall orchestration budget rather than a per-tool watchdog.
@@ -100,9 +89,7 @@ export function toolCallNeedsApproval(
10089
}
10190
}
10291

103-
return (
104-
callRequiresFreshApproval(toolName, args) || !context.toolPermissions.autoAllowed.has(toolName)
105-
)
92+
return !context.toolPermissions.autoAllowed.has(toolName)
10693
}
10794

10895
function skipOutput(toolName: string) {
@@ -313,7 +300,6 @@ export function runGatedToolExecution(
313300
return { status: MothershipStreamV1ToolOutcome.success, message: output.message }
314301
}
315302

316-
toolCall.userApproved = true
317303
await emitApprovedCall(toolCallId, toolName, executor, args, options)
318304

319305
const execution = execute()

apps/sim/lib/copilot/request/types.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,6 @@ export interface ToolCallState {
4343
* for main-lane tool calls.
4444
*/
4545
parentToolCallId?: string
46-
/** Set only after the server-side permission gate approves this exact call. */
47-
userApproved?: boolean
4846
}
4947

5048
export type ToolCallResult<T = unknown> = ToolExecutionResult & {

apps/sim/lib/copilot/tool-executor/types.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ export interface ToolExecutionContext {
1313
runId?: string
1414
/** Stable identity of the individual tool call being executed. */
1515
toolCallId?: string
16-
/** True only after the server-side permission gate approved this exact call. */
17-
userApprovedToolCall?: boolean
1816
billingAttribution?: BillingAttributionSnapshot
1917
copilotToolExecution?: boolean
2018
requestMode?: string

apps/sim/lib/copilot/tools/handlers/deployment/context.test.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { describe, expect, it } from 'vitest'
55
import {
66
getCopilotDeploymentIdempotencyKey,
77
getHistoricalDeploymentAttemptError,
8-
getUnapprovedUndeployError,
98
} from '@/lib/copilot/tools/handlers/deployment/context'
109

1110
describe('getCopilotDeploymentIdempotencyKey', () => {
@@ -30,15 +29,6 @@ describe('getCopilotDeploymentIdempotencyKey', () => {
3029
})
3130
})
3231

33-
describe('getUnapprovedUndeployError', () => {
34-
it('fails closed unless the server approved this exact tool call', () => {
35-
expect(getUnapprovedUndeployError({ userApprovedToolCall: false })).toContain(
36-
'requires explicit approval'
37-
)
38-
expect(getUnapprovedUndeployError({ userApprovedToolCall: true })).toBeNull()
39-
})
40-
})
41-
4232
describe('getHistoricalDeploymentAttemptError', () => {
4333
it('requires a new tool call when the persisted attempt is no longer current', () => {
4434
expect(getHistoricalDeploymentAttemptError({ isCurrent: false }, 'redeploy')).toContain(

apps/sim/lib/copilot/tools/handlers/deployment/context.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,6 @@ export function getCopilotDeploymentIdempotencyKey(
2525
: `copilot:tool-call:${context.toolCallId}`
2626
}
2727

28-
/** Returns the error used when an undeploy did not receive per-call user approval. */
29-
export function getUnapprovedUndeployError(
30-
context: Pick<ToolExecutionContext, 'userApprovedToolCall'>
31-
): string | null {
32-
if (context.userApprovedToolCall === true) return null
33-
return 'Undeploy requires explicit approval for this exact interactive Copilot call. Never undeploy to recover a failed deploy or redeploy; a failed redeploy already leaves the prior live version active.'
34-
}
35-
3628
/** Rejects a replay whose persisted operation no longer describes production. */
3729
export function getHistoricalDeploymentAttemptError(
3830
attempt: DeploymentAttemptCurrentState | null | undefined,

apps/sim/lib/copilot/tools/handlers/deployment/deploy.test.ts

Lines changed: 59 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,35 @@
11
/**
22
* @vitest-environment node
33
*/
4+
import { dbChainMockFns, resetDbChainMock } from '@sim/testing'
45
import { beforeEach, describe, expect, it, vi } from 'vitest'
56

6-
const { mockEnsureWorkflowAccess, mockPerformFullDeploy, mockPerformFullUndeploy } = vi.hoisted(
7-
() => ({
8-
mockEnsureWorkflowAccess: vi.fn(),
9-
mockPerformFullDeploy: vi.fn(),
10-
mockPerformFullUndeploy: vi.fn(),
11-
})
12-
)
7+
const {
8+
mockCheckChatAccess,
9+
mockEnsureWorkflowAccess,
10+
mockPerformChatUndeploy,
11+
mockPerformDeleteWorkflowMcpTool,
12+
mockPerformFullDeploy,
13+
mockPerformFullUndeploy,
14+
} = vi.hoisted(() => ({
15+
mockCheckChatAccess: vi.fn(),
16+
mockEnsureWorkflowAccess: vi.fn(),
17+
mockPerformChatUndeploy: vi.fn(),
18+
mockPerformDeleteWorkflowMcpTool: vi.fn(),
19+
mockPerformFullDeploy: vi.fn(),
20+
mockPerformFullUndeploy: vi.fn(),
21+
}))
1322

1423
vi.mock('@/lib/workflows/orchestration', () => ({
1524
performChatDeploy: vi.fn(),
16-
performChatUndeploy: vi.fn(),
25+
performChatUndeploy: mockPerformChatUndeploy,
1726
performFullDeploy: mockPerformFullDeploy,
1827
performFullUndeploy: mockPerformFullUndeploy,
1928
}))
2029

2130
vi.mock('@/lib/mcp/orchestration', () => ({
2231
performCreateWorkflowMcpTool: vi.fn(),
23-
performDeleteWorkflowMcpTool: vi.fn(),
32+
performDeleteWorkflowMcpTool: mockPerformDeleteWorkflowMcpTool,
2433
performUpdateWorkflowMcpTool: vi.fn(),
2534
}))
2635

@@ -35,7 +44,7 @@ vi.mock('@/lib/mcp/workflow-tool-schema', () => ({
3544
}))
3645

3746
vi.mock('@/app/api/chat/utils', () => ({
38-
checkChatAccess: vi.fn(),
47+
checkChatAccess: mockCheckChatAccess,
3948
checkWorkflowAccessForChatCreation: vi.fn(),
4049
}))
4150

@@ -55,30 +64,16 @@ import {
5564
executeRedeploy,
5665
} from '@/lib/copilot/tools/handlers/deployment/deploy'
5766

58-
describe('executeDeployApi', () => {
67+
describe('deployment handlers', () => {
5968
beforeEach(() => {
6069
vi.clearAllMocks()
70+
resetDbChainMock()
6171
mockEnsureWorkflowAccess.mockResolvedValue({
6272
workflow: { id: 'workflow-1', workspaceId: 'workspace-1' },
6373
})
6474
})
6575

66-
it('refuses undeploy without approval for the exact tool call', async () => {
67-
const result = await executeDeployApi(
68-
{ workflowId: 'workflow-1', action: 'undeploy' },
69-
{
70-
userId: 'user-1',
71-
workflowId: 'workflow-1',
72-
toolCallId: 'call-1',
73-
userApprovedToolCall: false,
74-
}
75-
)
76-
77-
expect(result).toMatchObject({ success: false, error: expect.stringContaining('approval') })
78-
expect(mockPerformFullUndeploy).not.toHaveBeenCalled()
79-
})
80-
81-
it('allows an explicitly approved undeploy', async () => {
76+
it('undeploys the API without approval context when permission gating is disabled', async () => {
8277
mockPerformFullUndeploy.mockResolvedValue({ success: true })
8378

8479
const result = await executeDeployApi(
@@ -87,7 +82,6 @@ describe('executeDeployApi', () => {
8782
userId: 'user-1',
8883
workflowId: 'workflow-1',
8984
toolCallId: 'call-1',
90-
userApprovedToolCall: true,
9185
}
9286
)
9387

@@ -182,31 +176,62 @@ describe('executeDeployApi', () => {
182176
})
183177
})
184178

185-
it('refuses chat undeploy without exact-call approval', async () => {
179+
it('undeploys chat without approval context when permission gating is disabled', async () => {
180+
dbChainMockFns.limit.mockResolvedValueOnce([
181+
{
182+
id: 'chat-1',
183+
identifier: 'production-helper',
184+
title: 'Production Helper',
185+
description: null,
186+
authType: 'public',
187+
allowedEmails: [],
188+
outputConfigs: [],
189+
includeThinking: false,
190+
includeToolCalls: false,
191+
customizations: null,
192+
},
193+
])
194+
mockCheckChatAccess.mockResolvedValue({ hasAccess: true, workspaceId: 'workspace-1' })
195+
mockPerformChatUndeploy.mockResolvedValue({ success: true })
196+
186197
const result = await executeDeployChat(
187198
{ workflowId: 'workflow-1', action: 'undeploy' },
188199
{
189200
userId: 'user-1',
190201
workflowId: 'workflow-1',
191202
toolCallId: 'call-1',
192-
userApprovedToolCall: false,
193203
}
194204
)
195205

196-
expect(result).toMatchObject({ success: false, error: expect.stringContaining('approval') })
206+
expect(result.success).toBe(true)
207+
expect(mockPerformChatUndeploy).toHaveBeenCalledWith({
208+
chatId: 'chat-1',
209+
userId: 'user-1',
210+
workspaceId: 'workspace-1',
211+
})
197212
})
198213

199-
it('refuses MCP undeploy without exact-call approval', async () => {
214+
it('undeploys MCP without approval context when permission gating is disabled', async () => {
215+
dbChainMockFns.limit
216+
.mockResolvedValueOnce([{ id: 'server-1', name: 'Production MCP' }])
217+
.mockResolvedValueOnce([{ id: 'tool-1' }])
218+
mockPerformDeleteWorkflowMcpTool.mockResolvedValue({ success: true })
219+
200220
const result = await executeDeployMcp(
201221
{ workflowId: 'workflow-1', serverId: 'server-1', action: 'undeploy' },
202222
{
203223
userId: 'user-1',
204224
workflowId: 'workflow-1',
205225
toolCallId: 'call-1',
206-
userApprovedToolCall: false,
207226
}
208227
)
209228

210-
expect(result).toMatchObject({ success: false, error: expect.stringContaining('approval') })
229+
expect(result.success).toBe(true)
230+
expect(mockPerformDeleteWorkflowMcpTool).toHaveBeenCalledWith({
231+
serverId: 'server-1',
232+
toolId: 'tool-1',
233+
workspaceId: 'workspace-1',
234+
userId: 'user-1',
235+
})
211236
})
212237
})

apps/sim/lib/copilot/tools/handlers/deployment/deploy.ts

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,7 @@ import {
2828
} from '@/ee/access-control/utils/permission-check'
2929
import { ensureWorkflowAccess } from '../access'
3030
import type { DeployApiParams, DeployChatParams, DeployMcpParams } from '../param-types'
31-
import {
32-
getCopilotDeploymentIdempotencyKey,
33-
getHistoricalDeploymentAttemptError,
34-
getUnapprovedUndeployError,
35-
} from './context'
31+
import { getCopilotDeploymentIdempotencyKey, getHistoricalDeploymentAttemptError } from './context'
3632

3733
function buildWorkflowApiEndpoint(baseUrl: string, workflowId: string): string {
3834
return `${baseUrl}/api/workflows/${workflowId}/execute`
@@ -140,9 +136,6 @@ export async function executeDeployApi(
140136
)
141137

142138
if (action === 'undeploy') {
143-
const approvalError = getUnapprovedUndeployError(context)
144-
if (approvalError) return { success: false, error: approvalError }
145-
146139
const result = await performFullUndeploy({ workflowId, userId: context.userId })
147140
if (!result.success) {
148141
return { success: false, error: result.error || 'Failed to undeploy workflow' }
@@ -263,9 +256,6 @@ export async function executeDeployChat(
263256

264257
const action = params.action === 'undeploy' ? 'undeploy' : 'deploy'
265258
if (action === 'undeploy') {
266-
const approvalError = getUnapprovedUndeployError(context)
267-
if (approvalError) return { success: false, error: approvalError }
268-
269259
const baseUrl = getBaseUrl()
270260
const apiEndpoint = buildWorkflowApiEndpoint(baseUrl, workflowId)
271261
const apiConfig = buildWorkflowApiConfig(baseUrl, apiEndpoint)
@@ -554,10 +544,6 @@ export async function executeDeployMcp(
554544
if (!workflowId) {
555545
return { success: false, error: 'workflowId is required' }
556546
}
557-
if (params.action === 'undeploy') {
558-
const approvalError = getUnapprovedUndeployError(context)
559-
if (approvalError) return { success: false, error: approvalError }
560-
}
561547

562548
const { workflow: workflowRecord } = await ensureWorkflowAccess(
563549
workflowId,

0 commit comments

Comments
 (0)