diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts index bee6a3186d5..06e46c664a0 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts @@ -2,16 +2,24 @@ * @vitest-environment node */ import { envFlagsMock } from '@sim/testing' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { normalizeConditionRouterIds } from './builders' -const { mockValidateSelectorIds, mockGetModelOptions, mockGetCustomToolById, mockGetSkillById } = - vi.hoisted(() => ({ - mockValidateSelectorIds: vi.fn(), - mockGetModelOptions: vi.fn(() => []), - mockGetCustomToolById: vi.fn(), - mockGetSkillById: vi.fn(), - })) +const { + mockValidateSelectorIds, + mockGetModelOptions, + mockEnvFlags, + mockGetTool, + mockGetCustomToolById, + mockGetSkillById, +} = vi.hoisted(() => ({ + mockValidateSelectorIds: vi.fn(), + mockGetModelOptions: vi.fn(() => []), + mockEnvFlags: { isHosted: false }, + mockGetTool: vi.fn(), + mockGetCustomToolById: vi.fn(), + mockGetSkillById: vi.fn(), +})) const conditionBlockConfig = { type: 'condition', @@ -73,6 +81,53 @@ const canonicalCredBlockConfig = { ], } +// Mirrors video_generator_v3: routes provider -> tool; only video_falai has hosting. +const videoBlockConfig = { + type: 'video_generator_v3', + name: 'Video Generator', + outputs: {}, + subBlocks: [{ id: 'provider', type: 'dropdown' }], + tools: { + access: ['video_runway', 'video_falai'], + config: { + tool: (params: Record) => + params.provider === 'falai' ? 'video_falai' : 'video_runway', + }, + }, +} + +// A hosted block whose tool's managed key param is NOT named 'apiKey'. +const customKeyBlockConfig = { + type: 'custom_key_block', + name: 'Custom Key Block', + outputs: {}, + subBlocks: [{ id: 'serviceKey', type: 'short-input' }], + tools: { access: ['custom_key_tool'], config: { tool: () => 'custom_key_tool' } }, +} + +// Single tool with a per-provider `enabled` gate (mirrors image_generate, falai-only hosting). +const imageBlockConfig = { + type: 'image_generator_v2', + name: 'Image Generator', + outputs: {}, + subBlocks: [{ id: 'provider', type: 'dropdown' }], + tools: { access: ['image_generate'], config: { tool: () => 'image_generate' } }, +} + +// Tool registry stand-in for the hosted-tool tests. +const toolsByIdMock: Record = { + video_falai: { id: 'video_falai', hosting: { apiKeyParam: 'apiKey' } }, + video_runway: { id: 'video_runway' }, + custom_key_tool: { id: 'custom_key_tool', hosting: { apiKeyParam: 'serviceKey' } }, + image_generate: { + id: 'image_generate', + hosting: { + apiKeyParam: 'apiKey', + enabled: (p: Record) => p.provider === 'falai', + }, + }, +} + vi.mock('@/blocks/registry', () => ({ getBlock: (type: string) => type === 'condition' @@ -89,13 +144,23 @@ vi.mock('@/blocks/registry', () => ({ ? knowledgeBlockConfig : type === 'canonicalcred' ? canonicalCredBlockConfig - : undefined, + : type === 'video_generator_v3' + ? videoBlockConfig + : type === 'custom_key_block' + ? customKeyBlockConfig + : type === 'image_generator_v2' + ? imageBlockConfig + : undefined, })) vi.mock('@/blocks/utils', () => ({ getModelOptions: mockGetModelOptions, })) +vi.mock('@/tools/utils', () => ({ + getTool: mockGetTool, +})) + vi.mock('@/lib/copilot/validation/selector-validator', () => ({ validateSelectorIds: mockValidateSelectorIds, })) @@ -108,7 +173,12 @@ vi.mock('@/lib/workflows/skills/operations', () => ({ getSkillById: mockGetSkillById, })) -vi.mock('@/lib/core/config/env-flags', () => envFlagsMock) +vi.mock('@/lib/core/config/env-flags', () => ({ + ...envFlagsMock, + get isHosted() { + return mockEnvFlags.isHosted + }, +})) vi.mock('@/providers/utils', () => ({ getHostedModels: () => [], @@ -321,6 +391,187 @@ describe('preValidateCredentialInputs', () => { }) }) +describe('preValidateCredentialInputs (hosted-tool blocks)', () => { + beforeEach(() => { + vi.clearAllMocks() + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: [] }) + mockGetTool.mockImplementation((id: string) => toolsByIdMock[id]) + mockEnvFlags.isHosted = true + }) + + afterEach(() => { + mockEnvFlags.isHosted = false + }) + + const ctx = { userId: 'user-1', workspaceId: 'workspace-1' } + + it('strips apiKey when the block resolves to a hosted tool on hosted Sim', async () => { + const operations = [ + { + operation_type: 'add' as const, + block_id: 'video-1', + params: { + type: 'video_generator_v3', + inputs: { provider: 'falai', model: 'veo-3.1', apiKey: '{{FAL_API_KEY}}' }, + }, + }, + ] + + const result = await preValidateCredentialInputs(operations, ctx) + + expect(result.filteredOperations[0]?.params?.inputs?.apiKey).toBeUndefined() + expect(result.errors).toHaveLength(1) + expect(result.errors[0]).toMatchObject({ blockId: 'video-1', field: 'apiKey' }) + expect(result.errors[0]?.error).toContain('managed by Sim') + }) + + it('preserves apiKey when the resolved tool has no hosting (non-falai provider)', async () => { + const operations = [ + { + operation_type: 'add' as const, + block_id: 'video-1', + params: { + type: 'video_generator_v3', + inputs: { provider: 'runway', apiKey: 'user-runway-key' }, + }, + }, + ] + + const result = await preValidateCredentialInputs(operations, ctx) + + expect(result.filteredOperations[0]?.params?.inputs?.apiKey).toBe('user-runway-key') + expect(result.errors).toHaveLength(0) + }) + + it('resolves provider from existing block state for edit ops that only set apiKey', async () => { + const operations = [ + { + operation_type: 'edit' as const, + block_id: 'video-1', + params: { + type: 'video_generator_v3', + inputs: { apiKey: '{{FAL_API_KEY}}' }, + }, + }, + ] + const workflowState = { + blocks: { + 'video-1': { + type: 'video_generator_v3', + subBlocks: { provider: { value: 'falai' } }, + }, + }, + } + + const result = await preValidateCredentialInputs(operations, ctx, workflowState) + + expect(result.filteredOperations[0]?.params?.inputs?.apiKey).toBeUndefined() + expect(result.errors).toHaveLength(1) + }) + + it('strips apiKey on a hosted-tool block nested inside a loop', async () => { + const operations = [ + { + operation_type: 'add' as const, + block_id: 'loop-1', + params: { + type: 'loop', + inputs: {}, + nestedNodes: { + 'video-child': { + type: 'video_generator_v3', + inputs: { provider: 'falai', model: 'veo-3.1', apiKey: '{{FAL_API_KEY}}' }, + }, + }, + }, + }, + ] + + const result = await preValidateCredentialInputs(operations, ctx) + + const nested = result.filteredOperations[0]?.params?.nestedNodes as + | Record }> + | undefined + expect(nested?.['video-child']?.inputs?.apiKey).toBeUndefined() + expect(result.errors).toHaveLength(1) + expect(result.errors[0]).toMatchObject({ blockId: 'video-child', field: 'apiKey' }) + }) + + it("strips a hosted tool's key field even when it is not named apiKey", async () => { + const operations = [ + { + operation_type: 'add' as const, + block_id: 'custom-1', + params: { + type: 'custom_key_block', + inputs: { serviceKey: '{{SOME_SERVICE_KEY}}' }, + }, + }, + ] + + const result = await preValidateCredentialInputs(operations, ctx) + + expect(result.filteredOperations[0]?.params?.inputs?.serviceKey).toBeUndefined() + expect(result.errors).toHaveLength(1) + expect(result.errors[0]).toMatchObject({ blockId: 'custom-1', field: 'serviceKey' }) + }) + + it('preserves apiKey on self-hosted deployments (isHosted false)', async () => { + mockEnvFlags.isHosted = false + const operations = [ + { + operation_type: 'add' as const, + block_id: 'video-1', + params: { + type: 'video_generator_v3', + inputs: { provider: 'falai', model: 'veo-3.1', apiKey: '{{FAL_API_KEY}}' }, + }, + }, + ] + + const result = await preValidateCredentialInputs(operations, ctx) + + expect(result.filteredOperations[0]?.params?.inputs?.apiKey).toBe('{{FAL_API_KEY}}') + expect(result.errors).toHaveLength(0) + }) + + it('strips apiKey when the tool hosting enabled gate passes (image, falai)', async () => { + const operations = [ + { + operation_type: 'add' as const, + block_id: 'image-1', + params: { + type: 'image_generator_v2', + inputs: { provider: 'falai', apiKey: '{{FAL_API_KEY}}' }, + }, + }, + ] + + const result = await preValidateCredentialInputs(operations, ctx) + + expect(result.filteredOperations[0]?.params?.inputs?.apiKey).toBeUndefined() + expect(result.errors).toHaveLength(1) + }) + + it('preserves apiKey when the tool hosting enabled gate fails (image, non-falai)', async () => { + const operations = [ + { + operation_type: 'add' as const, + block_id: 'image-1', + params: { + type: 'image_generator_v2', + inputs: { provider: 'openai', apiKey: 'user-openai-key' }, + }, + }, + ] + + const result = await preValidateCredentialInputs(operations, ctx) + + expect(result.filteredOperations[0]?.params?.inputs?.apiKey).toBe('user-openai-key') + expect(result.errors).toHaveLength(0) + }) +}) + const CTX = { userId: 'user-1', workspaceId: 'workspace-1' } describe('validateWorkflowSelectorIds (credential inclusion)', () => { diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts index 2fb0a6de35b..76e214f6e34 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts @@ -16,6 +16,7 @@ import type { SubBlockConfig } from '@/blocks/types' import { getModelOptions } from '@/blocks/utils' import { EDGE, normalizeName } from '@/executor/constants' import { isKnownModelId, suggestModelIdsForUnknownModel } from '@/providers/models' +import { getTool } from '@/tools/utils' import { TRIGGER_RUNTIME_SUBBLOCK_IDS } from '@/triggers/constants' import type { EdgeHandleValidationResult, @@ -1213,7 +1214,10 @@ export async function collectUnresolvedAgentToolReferences( /** * Pre-validates credential and apiKey inputs in operations before they are applied. * - Validates oauth-input (credential) IDs are accessible to the user in the workflow workspace - * - Filters out apiKey inputs for hosted models when isHosted is true + * - Filters out apiKey inputs when isHosted is true and the key is platform-managed: either a + * hosted LLM model (model in getHostedModels) or a block whose active tool declares + * `hosting` (e.g. Fal-backed video/image generators) - the canonical signal also used by + * injectHostedKeyIfNeeded at execution * - Also validates credentials and apiKeys in nestedNodes (blocks inside loop/parallel) * Returns validation errors for any removed inputs. */ @@ -1242,7 +1246,9 @@ export async function preValidateCredentialInputs( operationIndex: number blockId: string blockType: string - model: string + fieldName: string + reason: 'hosted_model' | 'hosted_tool' + model?: string nestedBlockId?: string }> = [] @@ -1297,12 +1303,80 @@ export async function preValidateCredentialInputs( operationIndex: opIndex, blockId, blockType, + fieldName: 'apiKey', + reason: 'hosted_model', model: modelValue, nestedBlockId, }) } } + /** + * Collect inputs targeting a hosted tool's key param. `tool.hosting` is the canonical + * "Sim provides this key" signal — the same one injectHostedKeyIfNeeded uses at execution. It + * names the managed field (`apiKeyParam`) and gates per-provider (`enabled`). We resolve the + * tool the block's current inputs select (via the block's `tools.config.tool` selector), so + * multi-provider blocks (video routing falai -> video_falai) and per-provider gates + * (image_generate, falai-only) match execution exactly. The UI hides these fields, but the + * copilot can still author them, so strip them here. + */ + function collectHostedToolApiKeyInput( + blockConfig: ReturnType, + inputs: Record, + toolParams: Record, + opIndex: number, + blockId: string, + blockType: string, + nestedBlockId?: string + ) { + if (!isHosted || !blockConfig?.tools) return + + // Resolve which tool(s) the current inputs select. With a selector there is exactly one active + // tool; without one, every accessible tool is a candidate. + let candidateToolIds: string[] + const toolSelector = blockConfig.tools.config?.tool + if (toolSelector) { + try { + candidateToolIds = [toolSelector(toolParams)] + } catch { + return + } + } else { + candidateToolIds = blockConfig.tools.access ?? [] + } + + const managedFieldIds = new Set() + for (const toolId of candidateToolIds) { + const tool = getTool(toolId) + if (!tool?.hosting) continue + if (tool.hosting.enabled && !tool.hosting.enabled(toolParams)) continue + managedFieldIds.add(tool.hosting.apiKeyParam) + } + + for (const fieldId of managedFieldIds) { + const value = inputs[fieldId] + if (typeof value !== 'string' || value.trim() === '') continue + + const alreadyCollected = hostedApiKeyInputs.some( + (e) => + e.operationIndex === opIndex && + e.blockId === blockId && + e.nestedBlockId === nestedBlockId && + e.fieldName === fieldId + ) + if (alreadyCollected) continue + + hostedApiKeyInputs.push({ + operationIndex: opIndex, + blockId, + blockType, + fieldName: fieldId, + reason: 'hosted_tool', + nestedBlockId, + }) + } + } + operations.forEach((op, opIndex) => { // Process main block inputs if (op.params?.inputs && op.params?.type) { @@ -1344,6 +1418,31 @@ export async function preValidateCredentialInputs( op.block_id, op.params.type ) + + // The active tool depends on inputs (e.g. provider). On edit ops that don't restate + // provider, merge the existing block's subblock values so the tool selector and its + // `enabled` gate resolve correctly. + let toolParams = op.params.inputs as Record + if (op.operation_type === 'edit' && workflowState) { + const existingBlock = (workflowState.blocks as Record)?.[op.block_id] as + | Record + | undefined + const existingSubBlocks = existingBlock?.subBlocks as + | Record + | undefined + if (existingSubBlocks) { + toolParams = { ...buildSubBlockValues(existingSubBlocks), ...toolParams } + } + } + + collectHostedToolApiKeyInput( + blockConfig, + op.params.inputs as Record, + toolParams, + opIndex, + op.block_id, + op.params.type + ) } } @@ -1373,6 +1472,15 @@ export async function preValidateCredentialInputs( // Check for apiKey inputs on hosted models in nested block const modelValue = childInputs.model as string | undefined collectHostedApiKeyInput(childInputs, modelValue, opIndex, op.block_id, childType, childId) + collectHostedToolApiKeyInput( + childBlockConfig, + childInputs, + childInputs, + opIndex, + op.block_id, + childType, + childId + ) }) } }) @@ -1389,10 +1497,16 @@ export async function preValidateCredentialInputs( // Filter out apiKey inputs for hosted models and add validation errors if (hasHostedApiKeysToFilter) { - logger.info('Filtering apiKey inputs for hosted models', { count: hostedApiKeyInputs.length }) + logger.info('Filtering platform-managed apiKey inputs', { count: hostedApiKeyInputs.length }) + + const stripMessage = (input: (typeof hostedApiKeyInputs)[number]): string => + input.reason === 'hosted_tool' + ? `Cannot set "${input.fieldName}" for "${input.blockType}" - it is managed by Sim on the hosted platform. Leave "${input.fieldName}" unset.` + : `Cannot set API key for hosted model "${input.model}" - API keys are managed by the platform when using hosted models` for (const apiKeyInput of hostedApiKeyInputs) { const op = filteredOperations[apiKeyInput.operationIndex] + const field = apiKeyInput.fieldName // Handle nested block apiKey filtering if (apiKeyInput.nestedBlockId) { @@ -1401,36 +1515,40 @@ export async function preValidateCredentialInputs( | undefined const nestedBlock = nestedNodes?.[apiKeyInput.nestedBlockId] const nestedInputs = nestedBlock?.inputs as Record | undefined - if (nestedInputs?.apiKey) { - nestedInputs.apiKey = undefined - logger.debug('Filtered apiKey for hosted model in nested block', { + if (nestedInputs?.[field]) { + nestedInputs[field] = undefined + logger.debug('Filtered platform-managed apiKey in nested block', { parentBlockId: apiKeyInput.blockId, nestedBlockId: apiKeyInput.nestedBlockId, + field, + reason: apiKeyInput.reason, model: apiKeyInput.model, }) errors.push({ blockId: apiKeyInput.nestedBlockId, blockType: apiKeyInput.blockType, - field: 'apiKey', + field, value: '[redacted]', - error: `Cannot set API key for hosted model "${apiKeyInput.model}" - API keys are managed by the platform when using hosted models`, + error: stripMessage(apiKeyInput), }) } - } else if (op.params?.inputs?.apiKey) { + } else if (op.params?.inputs?.[field]) { // Handle main block apiKey filtering - op.params.inputs.apiKey = undefined - logger.debug('Filtered apiKey for hosted model', { + op.params.inputs[field] = undefined + logger.debug('Filtered platform-managed apiKey', { blockId: apiKeyInput.blockId, + field, + reason: apiKeyInput.reason, model: apiKeyInput.model, }) errors.push({ blockId: apiKeyInput.blockId, blockType: apiKeyInput.blockType, - field: 'apiKey', + field, value: '[redacted]', - error: `Cannot set API key for hosted model "${apiKeyInput.model}" - API keys are managed by the platform when using hosted models`, + error: stripMessage(apiKeyInput), }) } }