fix(tools): reject a direct call missing a required user-only input

`validateRequiredParametersAfterMerge` checks `user-or-llm` parameters alone,
because on the workflow path a `user-only` parameter was already validated
during serialization against the block field that holds it. The v2 execute path
has no serialization step, so nothing had checked them: omitting
`zendesk_get_ticket`'s `subdomain` reached Zendesk as `undefined` and came back
a provider authentication failure — the same undiagnosable shape this branch
set out to remove.

The check exempts a parameter Sim supplies itself, mirroring
`injectHostedKeyIfNeeded`'s three tests in the same order so the two cannot
disagree about whether a value is coming. `firecrawl_scrape` stays callable
with no `apiKey` where keys are hosted, and requires one where they are not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: icecrasher321

apps/sim/lib/tool-execution/application/execute-tool.test.ts

@@ -2,7 +2,8 @@
  * @vitest-environment node
  */
 import type { PersonalApiKeyPrincipal, WorkspaceApiKeyPrincipal } from '@sim/auth/principal'
-import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { resetEnvFlagsMock, setEnvFlags } from '@sim/testing/mocks'
+import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
 
 const mocks = vi.hoisted(() => ({
   loadWorkspace: vi.fn(),
@@ -69,8 +70,8 @@ vi.mock('@/blocks/registry', () => ({
   getBlockMeta: vi.fn(() => ({ tags: [] })),
 }))
 
-vi.mock('@/tools/metadata', () => ({
-  getToolMetadata: (toolId: string) =>
+vi.mock('@/tools/utils', () => ({
+  getTool: (toolId: string) =>
     Object.hasOwn(TOOL_METADATA, toolId) ? TOOL_METADATA[toolId] : undefined,
 }))
 
@@ -98,10 +99,27 @@ const TOOL_METADATA: Record<string, Record<string, unknown>> = {
   slack_message: {
     id: 'slack_message',
     name: 'Slack Send Message',
-    params: { text: { type: 'string', required: true } },
+    params: { text: { type: 'string', required: true, visibility: 'user-or-llm' } },
     oauth: { required: true, provider: 'slack' },
   },
-  firecrawl_scrape: { id: 'firecrawl_scrape', name: 'Firecrawl Scrape', params: {} },
+  firecrawl_scrape: {
+    id: 'firecrawl_scrape',
+    name: 'Firecrawl Scrape',
+    params: {
+      url: { type: 'string', required: true, visibility: 'user-or-llm' },
+      apiKey: { type: 'string', required: true, visibility: 'user-only' },
+    },
+    hosting: { apiKeyParam: 'apiKey' },
+  },
+  zendesk_get_ticket: {
+    id: 'zendesk_get_ticket',
+    name: 'Zendesk Get Ticket',
+    params: {
+      subdomain: { type: 'string', required: true, visibility: 'user-only' },
+      apiToken: { type: 'string', required: true, visibility: 'user-only' },
+      ticketId: { type: 'string', required: true, visibility: 'user-or-llm' },
+    },
+  },
   preview_call: { id: 'preview_call', name: 'Preview Call', params: {} },
   confluence_read_v2: { id: 'confluence_read_v2', name: 'Confluence Read', params: {} },
 }
@@ -148,6 +166,7 @@ const previewBlock = block({
   preview: true,
   tools: { access: ['preview_call'] },
 })
+const zendeskBlock = block({ type: 'zendesk', tools: { access: ['zendesk_get_ticket'] } })
 const confluenceBlock = block({
   type: 'confluence_v2',
   tools: { access: ['confluence_read_v2'] },
@@ -156,20 +175,35 @@ const confluenceBlock = block({
 function run(input: Partial<Parameters<typeof executeToolForCaller.execute>[0]['input']> = {}) {
   return executeToolForCaller.execute({
     principal,
-    input: { workspaceId: WORKSPACE_ID, toolId: 'firecrawl_scrape', input: {}, ...input },
+    input: {
+      workspaceId: WORKSPACE_ID,
+      toolId: 'firecrawl_scrape',
+      input: { url: 'https://example.com' },
+      ...input,
+    },
   })
 }
 
 describe('executeToolForCaller', () => {
+  afterAll(resetEnvFlagsMock)
+
   beforeEach(() => {
     vi.clearAllMocks()
+    // Hosted-key injection only happens where Sim hosts keys.
+    setEnvFlags({ isHosted: true })
     mocks.loadWorkspace.mockResolvedValue(workspaceContext)
     mocks.resolvePermission.mockResolvedValue('write')
     mocks.allowedIntegrationTypes.mockResolvedValue(null)
     mocks.getBlockVisibility.mockResolvedValue({ revealed: new Set(), disabled: new Set() })
     mocks.listCustomBlocks.mockResolvedValue([])
     mocks.isDeploymentAvailable.mockReturnValue(true)
-    mocks.getAllBlocks.mockReturnValue([slackBlock, firecrawlBlock, previewBlock, confluenceBlock])
+    mocks.getAllBlocks.mockReturnValue([
+      slackBlock,
+      firecrawlBlock,
+      previewBlock,
+      confluenceBlock,
+      zendeskBlock,
+    ])
     mocks.executeRegistryTool.mockResolvedValue({ success: true, output: { markdown: '# Hi' } })
     mocks.resolveBillingAttribution.mockResolvedValue({ workspaceId: WORKSPACE_ID })
   })
@@ -247,6 +281,77 @@ describe('executeToolForCaller', () => {
     })
   })
 
+  /**
+   * The workflow path validates `user-only` parameters during serialization.
+   * This path has no serialization step, so without an explicit check a missing
+   * credential reached the provider as `undefined`.
+   */
+  it('refuses a missing required user-only input, naming every one of them', async () => {
+    await expect(
+      run({ toolId: 'zendesk_get_ticket', input: { ticketId: '42' } })
+    ).rejects.toMatchObject({
+      code: 'validation',
+      message: expect.stringContaining('input.subdomain'),
+    })
+    expect(mocks.executeRegistryTool).not.toHaveBeenCalled()
+  })
+
+  it('names the missing inputs together rather than one per round trip', async () => {
+    await expect(
+      run({ toolId: 'zendesk_get_ticket', input: { ticketId: '42' } })
+    ).rejects.toMatchObject({ message: expect.stringContaining('input.apiToken') })
+  })
+
+  it('treats a blank string as missing, the way the merge validator does', async () => {
+    await expect(
+      run({ toolId: 'zendesk_get_ticket', input: { ticketId: '4', subdomain: '', apiToken: 't' } })
+    ).rejects.toMatchObject({ code: 'validation' })
+  })
+
+  it('runs once every required user-only input is supplied', async () => {
+    await expect(
+      run({
+        toolId: 'zendesk_get_ticket',
+        input: { ticketId: '42', subdomain: 'acme', apiToken: 'tok' },
+      })
+    ).resolves.toMatchObject({ status: 'succeeded' })
+  })
+
+  /**
+   * `firecrawl_scrape` declares `apiKey` required and `user-only`, and Sim
+   * supplies it. Rejecting the omission would make every hosted-key tool
+   * uncallable without a key the caller does not need to have.
+   */
+  it('does not require a key the deployment hosts', async () => {
+    await expect(run({ input: { url: 'https://example.com' } })).resolves.toMatchObject({
+      status: 'succeeded',
+    })
+  })
+
+  /**
+   * Self-hosted supplies no hosted keys — `injectHostedKeyIfNeeded` short-circuits
+   * on `isHosted` — so the exemption must lift with it, or the caller is told a
+   * key is optional and the provider disagrees.
+   */
+  it('does require that key on a deployment that hosts none', async () => {
+    setEnvFlags({ isHosted: false })
+
+    await expect(run({ input: { url: 'https://example.com' } })).rejects.toMatchObject({
+      code: 'validation',
+      message: expect.stringContaining('input.apiKey'),
+    })
+  })
+
+  it('accepts a {{VAR}} reference as a present value, leaving resolution to the executor', async () => {
+    await run({
+      toolId: 'zendesk_get_ticket',
+      input: { ticketId: '4', subdomain: 'acme', apiToken: '{{ZENDESK_TOKEN}}' },
+    })
+
+    const [, params] = mocks.executeRegistryTool.mock.calls[0]
+    expect(params.apiToken).toBe('{{ZENDESK_TOKEN}}')
+  })
+
   it('requires a credential for an OAuth tool before it dispatches', async () => {
     await expect(run({ toolId: 'slack_message', input: { text: 'hi' } })).rejects.toMatchObject({
       code: 'validation',

apps/sim/lib/tool-execution/application/execute-tool.ts

@@ -13,11 +13,13 @@ import {
 } from '@/lib/catalog/application/tool-scope'
 import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application'
 import { ForbiddenOperationError } from '@/lib/core/application/forbidden'
+import { isHosted } from '@/lib/core/config/env-flags'
 import { OrchestrationError } from '@/lib/core/orchestration/types'
 import { principalUserId } from '@/lib/integrations/principal-scope.server'
 import { toolExecutionOperations } from '@/lib/tool-execution/application/operations'
 import { executeTool as executeRegistryTool } from '@/tools'
-import { getToolMetadata } from '@/tools/metadata'
+import type { ExecutableToolConfig } from '@/tools/types'
+import { getTool } from '@/tools/utils'
 
 const logger = createLogger('ExecuteToolUseCase')
 
@@ -79,6 +81,50 @@ function assertNoInlineCredential(args: Record<string, unknown>): void {
   }
 }
 
+/**
+ * Refuses a call missing a required parameter only its caller can supply.
+ *
+ * `validateRequiredParametersAfterMerge` deliberately checks `user-or-llm`
+ * alone, because on the workflow path a `user-only` parameter was already
+ * validated during serialization against the block field that holds it. This
+ * path has no serialization step, so nothing had checked them — a caller
+ * omitting `zendesk_get_ticket`'s `subdomain` reached Zendesk as `undefined`
+ * and came back a provider authentication failure, which is the same
+ * undiagnosable shape this PR set out to remove.
+ *
+ * A parameter Sim itself supplies is not missing: `hosting` fills its
+ * `apiKeyParam` on a key-hosting deployment, so `firecrawl_scrape` must stay
+ * callable with no `apiKey`. The condition mirrors `injectHostedKeyIfNeeded`
+ * exactly — same three tests, same order — so the two cannot disagree about
+ * whether a value is coming.
+ */
+function assertRequiredCallerInputsPresent(
+  tool: ExecutableToolConfig,
+  toolId: string,
+  params: Record<string, unknown>
+): void {
+  const hostedKeyParam =
+    isHosted && tool.hosting && (!tool.hosting.enabled || tool.hosting.enabled(params))
+      ? tool.hosting.apiKeyParam
+      : undefined
+
+  const missing = Object.entries(tool.params ?? {})
+    .filter(([name, declaration]) => {
+      if (!declaration?.required || declaration.visibility !== 'user-only') return false
+      if (name === hostedKeyParam) return false
+      const value = params[name]
+      return value === undefined || value === null || value === ''
+    })
+    .map(([name]) => name)
+
+  if (missing.length > 0) {
+    throw new OrchestrationError(
+      'validation',
+      `${toolId} requires ${missing.map((name) => `input.${name}`).join(', ')}`
+    )
+  }
+}
+
 /**
  * Runs one code-defined tool for an authenticated caller.
  *
@@ -136,13 +182,15 @@ export const executeToolForCaller = defineAuthorizedWorkspaceUseCase({
       )
     }
 
-    const metadata = getToolMetadata(toolId)
-    if (metadata?.oauth?.required && !input.credentialId) {
+    const tool = getTool(toolId)
+    if (!tool) throw new OrchestrationError('not_found', 'Tool not found')
+    if (tool.oauth?.required && !input.credentialId) {
       throw new OrchestrationError(
         'validation',
-        `credentialId is required: ${toolId} authenticates with a ${metadata.oauth.provider} credential`
+        `credentialId is required: ${toolId} authenticates with a ${tool.oauth.provider} credential`
       )
     }
+    assertRequiredCallerInputsPresent(tool, toolId, input.input)
 
     const userId = principalUserId(principal)
     if (!userId) {