Skip to content

Commit 818dca4

Browse files
committed
fix(agent): gate variable permission mode workflow writes
1 parent 64c7dd1 commit 818dca4

11 files changed

Lines changed: 304 additions & 7 deletions

File tree

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1582,8 +1582,7 @@ export const ToolInput = memo(function ToolInput({
15821582
const hasOperations =
15831583
!isCustomTool && !isMcpFamily && hasMultipleOperations(toolBlock ?? undefined)
15841584
const showToolControl = supportsToolControl && !(isMcpTool && isMcpToolUnavailable(tool))
1585-
const showCanonicalToolControl =
1586-
showToolControl && (permissionModeEnabled || toolUsageControlMode === 'advanced')
1585+
const showCanonicalToolControl = showToolControl && permissionModeEnabled
15871586
const hasToolBody =
15881587
showCanonicalToolControl || hasOperations || displaySubBlocks.length > 0
15891588

apps/sim/lib/core/config/feature-flags.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ interface FeatureFlagDefinition {
4848
const FEATURE_FLAGS = {
4949
'agent-tool-permission-mode': {
5050
description:
51-
'Enable the selector/variable Permission Mode editor for new agent tool configurations. ' +
52-
'Global on/off only; existing variable configurations remain editable and executable.',
51+
'Enable variable agent tool Permission Mode inputs in the editor and workflow writes. ' +
52+
'Global on/off only.',
5353
fallback: 'AGENT_TOOL_PERMISSION_MODE',
5454
},
5555
'trigger-eu-region': {

apps/sim/lib/workflows/application/apply-workflow-operations.test.ts

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22
* @vitest-environment node
33
*/
44
import { WorkflowLockedError } from '@sim/platform-authz/workflow'
5-
import { workflowAuthzMockFns } from '@sim/testing'
5+
import { createAgentBlock, workflowAuthzMockFns } from '@sim/testing'
66
import { beforeEach, describe, expect, it, vi } from 'vitest'
77

88
const mocks = vi.hoisted(() => ({
9+
isFeatureEnabled: vi.fn(),
910
recordAudit: vi.fn(),
1011
resolveContext: vi.fn(),
1112
resolvePermission: vi.fn(),
@@ -26,6 +27,10 @@ const mocks = vi.hoisted(() => ({
2627
collectGraphIds: vi.fn(),
2728
}))
2829

30+
vi.mock('@/lib/core/config/feature-flags', () => ({
31+
isFeatureEnabled: mocks.isFeatureEnabled,
32+
}))
33+
2934
vi.mock('@sim/audit', () => ({
3035
AuditAction: { WORKFLOW_UPDATED: 'workflow.updated' },
3136
AuditResourceType: { WORKFLOW: 'workflow' },
@@ -171,6 +176,7 @@ const GRAPH_IDS = { blockIds: ['block-1'], edgeIds: [], subflowIds: [] }
171176
describe('applyWorkflowOperations', () => {
172177
beforeEach(() => {
173178
vi.clearAllMocks()
179+
mocks.isFeatureEnabled.mockResolvedValue(false)
174180
mocks.resolveContext.mockResolvedValue(context)
175181
mocks.resolvePermission.mockResolvedValue('write')
176182
workflowAuthzMockFns.mockAssertWorkflowMutable.mockResolvedValue(undefined)
@@ -194,6 +200,52 @@ describe('applyWorkflowOperations', () => {
194200
mocks.assertIdsUnclaimed.mockResolvedValue(undefined)
195201
})
196202

203+
it.each([
204+
{
205+
principal: { kind: 'personal_api_key' as const, userId: 'user-1', keyId: 'key-1' },
206+
dryRun: false,
207+
},
208+
{
209+
principal: { kind: 'personal_api_key' as const, userId: 'user-1', keyId: 'key-1' },
210+
dryRun: true,
211+
},
212+
{ principal: copilotPrincipal, dryRun: false },
213+
{ principal: copilotPrincipal, dryRun: true },
214+
])(
215+
'gates variable mode edits for $principal.kind (dryRun=$dryRun)',
216+
async ({ principal, dryRun }) => {
217+
const agent = createAgentBlock({
218+
id: 'agent',
219+
subBlocks: {
220+
tools: {
221+
id: 'tools',
222+
type: 'tool-input',
223+
value: [{ type: 'function', usageControlExpression: '<start.toolMode>' }],
224+
},
225+
},
226+
data: { canonicalModes: { '0:agentToolUsageControl': 'advanced' } },
227+
})
228+
mocks.applyOperations.mockReturnValue({
229+
state: graph({ agent }),
230+
validationErrors: [],
231+
skippedItems: [],
232+
})
233+
const input = { workflowId: 'workflow-1', operations, layout: 'none' as const, dryRun }
234+
await expect(applyWorkflowOperations.execute({ principal, input })).rejects.toMatchObject({
235+
code: 'validation',
236+
message: 'Variable agent tool permission modes are disabled',
237+
})
238+
expect(mocks.replace).not.toHaveBeenCalled()
239+
expect(mocks.recordAudit).not.toHaveBeenCalled()
240+
expect(mocks.notify).not.toHaveBeenCalled()
241+
242+
mocks.isFeatureEnabled.mockResolvedValue(true)
243+
await expect(applyWorkflowOperations.execute({ principal, input })).resolves.toMatchObject({
244+
dryRun,
245+
})
246+
}
247+
)
248+
197249
it('writes once, through the shared persistence primitive', async () => {
198250
const result = await applyWorkflowOperations.execute({
199251
principal: sessionPrincipal,

apps/sim/lib/workflows/application/apply-workflow-operations.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import {
5454
} from '@/lib/workflows/persistence/replace-normalized-state'
5555
import { loadWorkflowFromNormalizedTables } from '@/lib/workflows/persistence/utils'
5656
import { validateWorkflowState } from '@/lib/workflows/sanitization/validation'
57+
import { assertAgentToolPermissionModeEnabled } from '@/lib/workflows/tool-input/usage-control.server'
5758
import { withBlockVisibility } from '@/blocks/visibility/server-context'
5859
import { generateLoopBlocks, generateParallelBlocks } from '@/stores/workflows/workflow/utils'
5960
import { normalizeWorkflowState } from '@/stores/workflows/workflow/validation'
@@ -347,6 +348,7 @@ export const applyWorkflowOperations = defineAuthorizedWorkflowUseCase({
347348
loops: generateLoopBlocks(blocks),
348349
parallels: generateParallelBlocks(blocks),
349350
}
351+
await assertAgentToolPermissionModeEnabled(Object.values(graph.blocks))
350352

351353
/**
352354
* Linted on the graph that is about to be persisted, so every finding

apps/sim/lib/workflows/application/replace-workflow-state.test.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22
* @vitest-environment node
33
*/
44
import { WorkflowLockedError } from '@sim/platform-authz/workflow'
5-
import { workflowAuthzMockFns } from '@sim/testing'
5+
import { createAgentBlock, workflowAuthzMockFns } from '@sim/testing'
66
import { beforeEach, describe, expect, it, vi } from 'vitest'
77

88
const mocks = vi.hoisted(() => ({
9+
isFeatureEnabled: vi.fn(),
910
recordAudit: vi.fn(),
1011
resolveContext: vi.fn(),
1112
resolvePermission: vi.fn(),
@@ -18,6 +19,10 @@ const mocks = vi.hoisted(() => ({
1819
needsRedeployment: vi.fn(),
1920
}))
2021

22+
vi.mock('@/lib/core/config/feature-flags', () => ({
23+
isFeatureEnabled: mocks.isFeatureEnabled,
24+
}))
25+
2126
vi.mock('@sim/audit', () => ({
2227
AuditAction: { WORKFLOW_UPDATED: 'workflow.updated' },
2328
AuditResourceType: { WORKFLOW: 'workflow' },
@@ -87,6 +92,7 @@ const input = { workflowId: 'workflow-1', blocks: { 'block-1': BLOCK }, edges: [
8792
describe('replaceWorkflowState', () => {
8893
beforeEach(() => {
8994
vi.clearAllMocks()
95+
mocks.isFeatureEnabled.mockResolvedValue(false)
9096
mocks.resolveContext.mockResolvedValue(context)
9197
mocks.resolvePermission.mockResolvedValue('write')
9298
workflowAuthzMockFns.mockAssertWorkflowMutable.mockResolvedValue(undefined)
@@ -104,6 +110,42 @@ describe('replaceWorkflowState', () => {
104110
mocks.assertIdsUnclaimed.mockResolvedValue(undefined)
105111
})
106112

113+
it.each([false, true])(
114+
'gates variable tool modes before a replacement (dryRun=%s)',
115+
async (dryRun) => {
116+
const agent = createAgentBlock({
117+
id: 'agent',
118+
subBlocks: {
119+
tools: {
120+
id: 'tools',
121+
type: 'tool-input',
122+
value: [{ type: 'function', usageControlExpression: '<start.toolMode>' }],
123+
},
124+
},
125+
})
126+
await expect(
127+
replaceWorkflowState.execute({
128+
principal: sessionPrincipal,
129+
input: { ...input, blocks: { agent }, dryRun },
130+
})
131+
).rejects.toMatchObject({
132+
code: 'validation',
133+
message: 'Variable agent tool permission modes are disabled',
134+
})
135+
expect(mocks.replace).not.toHaveBeenCalled()
136+
expect(mocks.recordAudit).not.toHaveBeenCalled()
137+
expect(mocks.notify).not.toHaveBeenCalled()
138+
139+
mocks.isFeatureEnabled.mockResolvedValue(true)
140+
await expect(
141+
replaceWorkflowState.execute({
142+
principal: sessionPrincipal,
143+
input: { ...input, blocks: { agent }, dryRun },
144+
})
145+
).resolves.toMatchObject({ dryRun })
146+
}
147+
)
148+
107149
/**
108150
* Two things this pins that a same-shape input and output cannot: the write
109151
* carries the **sanitized** graph, not the caller's body, and the reported

apps/sim/lib/workflows/application/replace-workflow-state.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
replaceWorkflowNormalizedState,
2828
} from '@/lib/workflows/persistence/replace-normalized-state'
2929
import { validateWorkflowState } from '@/lib/workflows/sanitization/validation'
30+
import { assertAgentToolPermissionModeEnabled } from '@/lib/workflows/tool-input/usage-control.server'
3031

3132
const logger = createLogger('ReplaceWorkflowState')
3233

@@ -127,6 +128,7 @@ export const replaceWorkflowState = defineAuthorizedWorkflowUseCase({
127128
blocks: sanitized.blocks as Record<string, BlockState>,
128129
edges: sanitized.edges as WorkflowState['edges'],
129130
}
131+
await assertAgentToolPermissionModeEnabled(Object.values(graph.blocks))
130132

131133
/**
132134
* Linted before the write so a dry run and a committed write report the

apps/sim/lib/workflows/operations/import-workflow.test.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { queueTableRows, resetDbChainMock, schemaMock } from '@sim/testing'
4+
import { createAgentBlock, queueTableRows, resetDbChainMock, schemaMock } from '@sim/testing'
55
import { beforeEach, describe, expect, it, vi } from 'vitest'
66

77
const mocks = vi.hoisted(() => ({
8+
isFeatureEnabled: vi.fn(),
89
getUserPermissionConfig: vi.fn(),
910
performCreateWorkflow: vi.fn(),
1011
performCreateWorkflowTransition: vi.fn(),
1112
saveWorkflowToNormalizedTables: vi.fn(),
1213
extractAndPersistCustomTools: vi.fn(),
1314
}))
1415

16+
vi.mock('@/lib/core/config/feature-flags', () => ({
17+
isFeatureEnabled: mocks.isFeatureEnabled,
18+
}))
19+
1520
vi.mock('@/lib/permission-groups/resolve.server', () => ({
1621
getUserPermissionConfig: mocks.getUserPermissionConfig,
1722
}))
@@ -62,6 +67,7 @@ function params(workflowPayload: Record<string, unknown>) {
6267
describe('importWorkflowIntoWorkspace block access', () => {
6368
beforeEach(() => {
6469
vi.clearAllMocks()
70+
mocks.isFeatureEnabled.mockResolvedValue(false)
6571
resetDbChainMock()
6672
queueTableRows(schemaMock.workspace, [{ id: 'workspace-1' }])
6773
mocks.getUserPermissionConfig.mockResolvedValue(null)
@@ -81,6 +87,35 @@ describe('importWorkflowIntoWorkspace block access', () => {
8187
mocks.extractAndPersistCustomTools.mockResolvedValue({ saved: 0, errors: [] })
8288
})
8389

90+
it.each([false, true])(
91+
'gates variable modes before creating an import (enabled=%s)',
92+
async (enabled) => {
93+
mocks.isFeatureEnabled.mockResolvedValue(enabled)
94+
const agent = createAgentBlock({
95+
id: 'agent',
96+
subBlocks: {
97+
tools: {
98+
id: 'tools',
99+
type: 'tool-input',
100+
value: [{ type: 'function', usageControlExpression: '<start.toolMode>' }],
101+
},
102+
},
103+
})
104+
const result = importWorkflowIntoWorkspace(params(payload(agent)))
105+
if (enabled) {
106+
await expect(result).resolves.toMatchObject({ success: true })
107+
expect(mocks.performCreateWorkflow).toHaveBeenCalledOnce()
108+
} else {
109+
await expect(result).rejects.toMatchObject({
110+
code: 'validation',
111+
message: 'Variable agent tool permission modes are disabled',
112+
})
113+
expect(mocks.performCreateWorkflow).not.toHaveBeenCalled()
114+
expect(mocks.saveWorkflowToNormalizedTables).not.toHaveBeenCalled()
115+
}
116+
}
117+
)
118+
84119
/**
85120
* The bypass this closes: import never went through the editing operations,
86121
* so a denied integration reached the normalized tables and was refused only

apps/sim/lib/workflows/operations/import-workflow.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
import { extractAndPersistCustomTools } from '@/lib/workflows/persistence/custom-tools-persistence'
3131
import { prepareWorkflowStateForPersistence } from '@/lib/workflows/persistence/prepare-state'
3232
import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils'
33+
import { assertAgentToolPermissionModeEnabled } from '@/lib/workflows/tool-input/usage-control.server'
3334
import { normalizeImportedVariables } from '@/lib/workflows/variables/parse'
3435
import type { WorkflowState } from '@/stores/workflows/workflow/types'
3536

@@ -273,6 +274,7 @@ async function executeImportWorkflowIntoWorkspace(
273274
}
274275

275276
const workflowState: WorkflowState = { ...parsedState, ...preparedState }
277+
await assertAgentToolPermissionModeEnabled(Object.values(workflowState.blocks))
276278

277279
/**
278280
* Nothing has been written yet, which is why the check sits here: an import

0 commit comments

Comments
 (0)