Skip to content

Commit 8d3b922

Browse files
committed
fix(access-control): gate the seeded model too, and collapse the seed rule
Block creation seeds `model` the same way it seeds `operation`: `agent`, `router` and `evaluator` all declare `defaultValue: 'claude-sonnet-5'`, and `prepareBlockState` wrote it unconditionally. The model combobox only fills a field that is empty, so a group denying that model (or the Anthropic provider) got an Agent block pre-filled with a model it cannot run — the same bug as the operation one, on the other axis the permission group governs. Rather than a second bespoke gate, `prepareBlockState` now takes one veto, `isSeededValueAllowed(subBlockId, value)`, and seeds nothing when a declared default is denied. Nothing substitutes a replacement there any more: the editor's own permission-aware pickers already resolve the right one and only fill an empty field, and substituting in the store would drift from `getDefaultBlockName`, which names a block after its *declared* default. That also deletes `firstAllowedOperation` and its copy of subblock-option enumeration. Review follow-ups: - `usePermissionConfig` gains `isModelUsable` (denylist AND provider allowlist); the combobox's two hand-rolled copies of that pair now call it - `isToolAllowed`/`isModelAllowed` index their denylists — the gate calls them once per option of every block offered, so a linear scan made a check's cost scale with denylist length (measured 3.2ms -> 0.30ms per search-index build at 500 denied tools) - `useOperationAccess` had three members with three different loading semantics, one documented as unsafe alone; it now exposes one withholding `resolveOperationGate` - the agent tool picker derived its option list twice with the empty-id filter on only one path; both callers now share one `{ options, denied }` result - `OPERATION_SUBBLOCK_ID` was a verbatim copy of the private constant in `canvas-sentence.ts`, doc comment included; that file now imports it
1 parent 310b4be commit 8d3b922

10 files changed

Lines changed: 233 additions & 171 deletions

File tree

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

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import { useAccessibleReferencePrefixes } from '@/app/workspace/[workspaceId]/w/
1515
import type { SubBlockConfig } from '@/blocks/types'
1616
import { getDependsOnFields } from '@/blocks/utils'
1717
import { usePermissionConfig } from '@/hooks/use-permission-config'
18-
import { getProviderFromModel } from '@/providers/utils'
1918
import { useSubBlockStore } from '@/stores/workflows/subblock/store'
2019

2120
/**
@@ -108,30 +107,18 @@ export const ComboBox = memo(function ComboBox({
108107
const value = isPreview ? previewValue : propValue !== undefined ? propValue : storeValue
109108

110109
// Permission-based filtering for model dropdowns
111-
const {
112-
isProviderAllowed,
113-
isModelAllowed,
114-
isLoading: isPermissionLoading,
115-
} = usePermissionConfig()
110+
const { isModelUsable, isLoading: isPermissionLoading } = usePermissionConfig()
116111

117112
// Evaluate static options if provided as a function
118113
const staticOptions = useMemo(() => {
119114
const opts = typeof options === 'function' ? options() : options
120115

121116
if (subBlockId === 'model') {
122-
return opts.filter((opt) => {
123-
const modelId = typeof opt === 'string' ? opt : opt.id
124-
if (!isModelAllowed(modelId)) return false
125-
try {
126-
return isProviderAllowed(getProviderFromModel(modelId))
127-
} catch {
128-
return true
129-
}
130-
})
117+
return opts.filter((opt) => isModelUsable(typeof opt === 'string' ? opt : opt.id))
131118
}
132119

133120
return opts
134-
}, [options, subBlockId, isProviderAllowed, isModelAllowed])
121+
}, [options, subBlockId, isModelUsable])
135122

136123
const {
137124
fetchedOptions,
@@ -210,15 +197,7 @@ export const ComboBox = memo(function ComboBox({
210197
fetchOptions && normalizedFetchedOptions.length > 0 ? normalizedFetchedOptions : staticOptions
211198

212199
if (subBlockId === 'model' && fetchOptions && normalizedFetchedOptions.length > 0) {
213-
opts = opts.filter((opt) => {
214-
const modelId = typeof opt === 'string' ? opt : opt.id
215-
if (!isModelAllowed(modelId)) return false
216-
try {
217-
return isProviderAllowed(getProviderFromModel(modelId))
218-
} catch {
219-
return true
220-
}
221-
})
200+
opts = opts.filter((opt) => isModelUsable(typeof opt === 'string' ? opt : opt.id))
222201
}
223202

224203
// Merge hydrated option if not already present
@@ -251,8 +230,7 @@ export const ComboBox = memo(function ComboBox({
251230
hydratedOption,
252231
createdOption,
253232
subBlockId,
254-
isProviderAllowed,
255-
isModelAllowed,
233+
isModelUsable,
256234
])
257235

258236
// Convert options to Combobox format

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ import { memo, useCallback, useEffect, useMemo, useRef } from 'react'
22
import { ChipTag, Combobox, type ComboboxOption } from '@sim/emcn'
33
import { generateId } from '@sim/utils/id'
44
import { isRecordLike } from '@sim/utils/object'
5-
import { OPERATION_SUBBLOCK_ID } from '@/lib/permission-groups/operation-access'
5+
import {
6+
NO_DENIED_OPERATIONS,
7+
OPERATION_SUBBLOCK_ID,
8+
} from '@/lib/permission-groups/operation-access'
69
import { formatDisplayText } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/formatted-text'
710
import { getWorkflowSearchLabelHighlight } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/workflow-search-highlight'
811
import { useFetchedOptions } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-fetched-options'
@@ -18,8 +21,6 @@ import { useWorkflowStore } from '@/stores/workflows/workflow/store'
1821
/** Selected-value badges shown before folding the rest into a "+N" badge. */
1922
const MAX_VISIBLE_MULTI_SELECT_BADGES = 2
2023

21-
const EMPTY_DENIED_OPERATIONS: ReadonlySet<string> = new Set()
22-
2324
/**
2425
* Dropdown option type - can be a simple string or an object with label, id, and optional icon.
2526
* Options with `hidden: true` are excluded from the picker but still resolve for label display,
@@ -197,7 +198,7 @@ export const Dropdown = memo(function Dropdown({
197198
* authoritative gate regardless.
198199
*/
199200
const deniedOperationIds = useMemo(() => {
200-
if (subBlockId !== OPERATION_SUBBLOCK_ID) return EMPTY_DENIED_OPERATIONS
201+
if (subBlockId !== OPERATION_SUBBLOCK_ID) return NO_DENIED_OPERATIONS
201202
return getDeniedOperations(
202203
blockConfig,
203204
allOptions.map((opt) => (typeof opt === 'string' ? opt : opt.id))
@@ -226,8 +227,6 @@ export const Dropdown = memo(function Dropdown({
226227
const defaultOptionValue = useMemo(() => {
227228
if (multiSelect) return undefined
228229

229-
const selectableIds = comboboxOptions.filter((opt) => !opt.hidden).map((opt) => opt.value)
230-
231230
/**
232231
* The operation field defaults through the permission gate, which withholds
233232
* a value until the group config has loaded. Seeding the static first
@@ -236,12 +235,13 @@ export const Dropdown = memo(function Dropdown({
236235
* that arrives with the config would never apply.
237236
*/
238237
if (subBlockId === OPERATION_SUBBLOCK_ID) {
238+
const selectableIds = comboboxOptions.filter((opt) => !opt.hidden).map((opt) => opt.value)
239239
return resolveDefaultOperation(blockConfig, selectableIds, defaultValue)
240240
}
241241

242242
if (defaultValue !== undefined) return defaultValue
243243

244-
return selectableIds[0]
244+
return comboboxOptions.find((opt) => !opt.hidden)?.value
245245
}, [defaultValue, comboboxOptions, multiSelect, subBlockId, blockConfig, resolveDefaultOperation])
246246

247247
useEffect(() => {

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

Lines changed: 36 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ import {
2626
} from '@/lib/mcp/tool-validation'
2727
import type { McpToolSchema } from '@/lib/mcp/types'
2828
import { getProviderIdFromServiceId, type OAuthProvider, type OAuthService } from '@/lib/oauth'
29-
import { OPERATION_SUBBLOCK_ID } from '@/lib/permission-groups/operation-access'
29+
import {
30+
NO_DENIED_OPERATIONS,
31+
OPERATION_SUBBLOCK_ID,
32+
} from '@/lib/permission-groups/operation-access'
3033
import { extractInputFieldsFromBlocks } from '@/lib/workflows/input-format'
3134
import { resolveStoredToolName } from '@/lib/workflows/subblocks/display'
3235
import { buildToolSubBlockId } from '@/lib/workflows/tool-input/synthetic-subblocks'
@@ -666,18 +669,24 @@ export const ToolInput = memo(function ToolInput({
666669
const { getDeniedOperations } = useOperationAccess()
667670

668671
/**
669-
* A tool block's operations minus the ones the caller's permission group
670-
* denies, so this surface never offers — or silently defaults to — an
671-
* operation that would be rejected at execution.
672+
* A tool block's selectable operations paired with the ones the caller's
673+
* permission group denies.
674+
*
675+
* Both callers derive from this single result so they cannot drift: the
676+
* picker *removes* denied operations (it must never offer or default to one),
677+
* while the editor's selector *hides* them (a tool already saved on one keeps
678+
* showing its name).
672679
*/
673-
const getAllowedOperationOptions = useCallback(
680+
const getOperationChoices = useCallback(
674681
(block: BlockConfig | undefined) => {
675-
const options = getOperationOptions(block)
676-
const denied = getDeniedOperations(
677-
block,
678-
options.map((option) => option.id)
679-
)
680-
return denied.size === 0 ? options : options.filter((option) => !denied.has(option.id))
682+
const options = getOperationOptions(block).filter((option) => option.id !== '')
683+
return {
684+
options,
685+
denied: getDeniedOperations(
686+
block,
687+
options.map((option) => option.id)
688+
),
689+
}
681690
},
682691
[getDeniedOperations]
683692
)
@@ -688,10 +697,12 @@ export const ToolInput = memo(function ToolInput({
688697
/* A multi-operation block whose every operation is denied has nothing the
689698
caller can run, so it leaves the picker alongside the blocks denied
690699
outright by `filterBlocks`. */
691-
return filterBlocks(allToolBlocks).filter(
692-
(block) => !hasMultipleOperations(block) || getAllowedOperationOptions(block).length > 0
693-
)
694-
}, [filterBlocks, customBlockOverlayVersion, getAllowedOperationOptions])
700+
return filterBlocks(allToolBlocks).filter((block) => {
701+
if (!hasMultipleOperations(block)) return true
702+
const { options, denied } = getOperationChoices(block)
703+
return options.length === 0 || options.some((option) => !denied.has(option.id))
704+
})
705+
}, [filterBlocks, customBlockOverlayVersion, getOperationChoices])
695706

696707
const hasBackfilledRef = useRef(false)
697708
useEffect(() => {
@@ -806,9 +817,10 @@ export const ToolInput = memo(function ToolInput({
806817
(toolBlock: (typeof toolBlocks)[0]) => {
807818
if (isPreview || disabled) return
808819

809-
const hasOperations = hasMultipleOperations(toolBlock)
810-
const operationOptions = hasOperations ? getAllowedOperationOptions(toolBlock) : []
811-
const defaultOperation = operationOptions.length > 0 ? operationOptions[0].id : undefined
820+
const { options, denied } = hasMultipleOperations(toolBlock)
821+
? getOperationChoices(toolBlock ?? undefined)
822+
: { options: [], denied: NO_DENIED_OPERATIONS }
823+
const defaultOperation = options.find((option) => !denied.has(option.id))?.id
812824

813825
const toolId = getToolIdForOperation(toolBlock.type, defaultOperation, toolBlock)
814826
if (!toolId) return
@@ -844,14 +856,7 @@ export const ToolInput = memo(function ToolInput({
844856

845857
setOpen(false)
846858
},
847-
[
848-
isPreview,
849-
disabled,
850-
isToolAlreadySelected,
851-
selectedTools,
852-
setStoreValue,
853-
getAllowedOperationOptions,
854-
]
859+
[isPreview, disabled, isToolAlreadySelected, selectedTools, setStoreValue, getOperationChoices]
855860
)
856861

857862
const handleAddCustomTool = useCallback(
@@ -1830,7 +1835,7 @@ export const ToolInput = memo(function ToolInput({
18301835
: []
18311836

18321837
const hasOperations =
1833-
!isCustomTool && !isMcpTool && hasMultipleOperations(getBlock(tool.type))
1838+
!isCustomTool && !isMcpTool && hasMultipleOperations(toolBlock ?? undefined)
18341839
const hasParams = useSubBlocks
18351840
? displaySubBlocks.length > 0
18361841
: displayParams.filter((param) => evaluateParameterCondition(param, tool)).length > 0
@@ -2090,19 +2095,11 @@ export const ToolInput = memo(function ToolInput({
20902095
<div className='flex flex-col gap-2.5 overflow-visible rounded-b-[4px] border-[var(--border-1)] border-t bg-[var(--surface-2)] p-2'>
20912096
{/* Operation dropdown for tools with multiple operations */}
20922097
{(() => {
2093-
const block = getBlock(tool.type)
2094-
const operationOptions = hasMultipleOperations(block)
2095-
? getOperationOptions(block).filter((option) => option.id !== '')
2096-
: []
2097-
if (operationOptions.length === 0) return null
2098-
2099-
/* Denied operations are hidden from the picker rather than
2100-
dropped, so a tool already saved on one keeps showing its
2101-
name; the unset fallback skips to the first allowed. */
2102-
const denied = getDeniedOperations(
2103-
block,
2104-
operationOptions.map((option) => option.id)
2098+
if (!hasOperations) return null
2099+
const { options: operationOptions, denied } = getOperationChoices(
2100+
toolBlock ?? undefined
21052101
)
2102+
if (operationOptions.length === 0) return null
21062103

21072104
return (
21082105
<div className='relative space-y-1.5'>

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ import { useSession } from '@/lib/auth/auth-client'
4141
import type { OAuthConnectEventDetail } from '@/lib/copilot/tools/client/base-tool'
4242
import { consumeOAuthReturnContext, writeOAuthReturnContext } from '@/lib/credentials/client-state'
4343
import type { OAuthProvider } from '@/lib/oauth'
44-
import { OPERATION_SUBBLOCK_ID } from '@/lib/permission-groups/operation-access'
44+
import { MODEL_SUBBLOCK_ID, OPERATION_SUBBLOCK_ID } from '@/lib/permission-groups/operation-access'
4545
import { getDefaultBlockName } from '@/lib/workflows/blocks/canvas-presentation'
4646
import { requestNoteImage, requestNoteRename } from '@/lib/workflows/notes/canvas-requests'
4747
import { TriggerUtils } from '@/lib/workflows/triggers/triggers'
@@ -129,6 +129,7 @@ import { useCanvasViewport } from '@/hooks/use-canvas-viewport'
129129
import { useCollaborativeWorkflow } from '@/hooks/use-collaborative-workflow'
130130
import { useOAuthReturnForWorkflow } from '@/hooks/use-oauth-return'
131131
import { useOperationAccess } from '@/hooks/use-operation-access'
132+
import { usePermissionConfig } from '@/hooks/use-permission-config'
132133
import { useCanvasModeStore } from '@/stores/canvas-mode'
133134
import { useChatStore } from '@/stores/chat/store'
134135
import {
@@ -870,7 +871,8 @@ const WorkflowContent = React.memo(
870871
*/
871872
const pendingFocusBlockIdRef = useRef<string | null>(null)
872873

873-
const { isOperationAllowed, isReady: isOperationAccessReady } = useOperationAccess()
874+
const { resolveOperationGate } = useOperationAccess()
875+
const { isModelUsable } = usePermissionConfig()
874876

875877
const addBlock = useCallback(
876878
(
@@ -894,11 +896,17 @@ const WorkflowContent = React.memo(
894896
if (extent) blockData.extent = extent
895897

896898
/**
897-
* Withheld until the permission config has resolved, since it reads as
898-
* "nothing denied" in flight and would let a denied default through.
899+
* `undefined` until the permission config has resolved, so a declared
900+
* default is never vetoed — or let through — on a guess. Blocks pre-fill
901+
* two fields the group can restrict; both go through the same gate.
899902
*/
900-
const operationGate = isOperationAccessReady
901-
? (operationId: string) => isOperationAllowed(getBlock(type), operationId)
903+
const operationGate = resolveOperationGate(getBlock(type))
904+
const seedGate = operationGate
905+
? (subBlockId: string, value: string) => {
906+
if (subBlockId === OPERATION_SUBBLOCK_ID) return operationGate(value)
907+
if (subBlockId === MODEL_SUBBLOCK_ID) return isModelUsable(value)
908+
return true
909+
}
902910
: undefined
903911

904912
const block = prepareBlockState({
@@ -910,7 +918,7 @@ const WorkflowContent = React.memo(
910918
parentId,
911919
extent,
912920
triggerMode,
913-
isOperationAllowed: operationGate,
921+
isSeededValueAllowed: seedGate,
914922
})
915923

916924
const subBlockValues: Record<string, Record<string, unknown>> = {}
@@ -958,8 +966,8 @@ const WorkflowContent = React.memo(
958966
collaborativeBatchAddBlocks,
959967
setSelectedEdges,
960968
setPendingSelection,
961-
isOperationAllowed,
962-
isOperationAccessReady,
969+
resolveOperationGate,
970+
isModelUsable,
963971
]
964972
)
965973

0 commit comments

Comments
 (0)