fix(copilot): strip platform-managed apiKey on hosted-tool blocks in edit_workflow (#5217)

preValidateCredentialInputs only stripped apiKey for hosted LLM models (getHostedModels). Agent-authored apiKey on hosted-tool blocks (Fal video/image, etc.) slipped through, disabling hosted-key injection and misleading the agent into telling users to bring a key. Generalize the strip to key off tool.hosting — the same canonical signal injectHostedKeyIfNeeded uses at execution. Resolve the block's active tool via its tools.config.tool selector, honor the per-provider enabled gate, and strip the exact hosting.apiKeyParam field (no hardcoded 'apiKey' assumption). Surfaces the existing non-fatal note to the agent. Self-hosted and non-hosted providers untouched.
2026-09-24 15:45:35 +08:00 · 2026-06-25 19:03:30 -04:00
parent d20deedf99
commit 5db5963fc1
2 changed files with 392 additions and 23 deletions
@@ -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<string, unknown>) =>
+        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<string, unknown> = {
+  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<string, unknown>) => 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<string, { inputs?: Record<string, unknown> }>
+      | 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)', () => {
@@ -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<typeof getBlock>,
+    inputs: Record<string, unknown>,
+    toolParams: Record<string, unknown>,
+    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<string>()
+    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<string, unknown>
+        if (op.operation_type === 'edit' && workflowState) {
+          const existingBlock = (workflowState.blocks as Record<string, unknown>)?.[op.block_id] as
+            | Record<string, unknown>
+            | undefined
+          const existingSubBlocks = existingBlock?.subBlocks as
+            | Record<string, { value?: unknown } | null | undefined>
+            | undefined
+          if (existingSubBlocks) {
+            toolParams = { ...buildSubBlockValues(existingSubBlocks), ...toolParams }
+          }
+        }
+
+        collectHostedToolApiKeyInput(
+          blockConfig,
+          op.params.inputs as Record<string, unknown>,
+          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<string, unknown> | 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),
        })
      }
    }