fix(tools): stop MCP pinned params from stating the server's identity

The MCP discovery path stripped only `toolName` from a tool entry's stored
params, so `serverId` and `serverName` stayed in the values treated as pinned
and were stated to the model — an internal server id reaching a provider, and
two of the three field slots consumed before any real param. The cached path
already stripped all three; the split now lives in one helper so the two cannot
drift again.

Pi withheld literal values for every tool whenever ANY input in the whole run
resolved a secret, which is true of almost any real workflow — the feature was
effectively off there. It now asks the registry the same per-input-path question
the Agent block asks, so only the tool that actually carries a secret is
withheld.

A canonical group blocked by one half no longer leaks through the other: a
`file-upload` basic half skipped without claiming its param, letting a
`short-input` twin state a raw file reference. 78 groups have that shape.

Widens the secret-name backstop for params named by a remote MCP schema rather
than by Sim — `authorization`, `cookie`, `signature`, `connectionString`, `otp`
and friends are not in the Sim-tuned `isPasswordParameter` list.

Author: waleedlatif1

apps/sim/executor/handlers/agent/agent-handler.ts

@@ -211,6 +211,19 @@ function isTransportTimeout(error: unknown): boolean {
 /**
  * Handler for Agent blocks that process LLM requests with optional tools.
  */
+/**
+ * Splits an MCP tool entry's stored params into the keys that identify the server and the values
+ * the user pinned on the call.
+ *
+ * Both MCP paths must strip the same control keys: they name the server rather than the request,
+ * and anything left in `userProvidedParams` is stated to the model as a pinned value. Keeping the
+ * split in one place is what stops the two paths drifting apart.
+ */
+function splitMcpControlParams(params: Record<string, any> | undefined) {
+  const { serverId, serverName, toolName, ...userProvidedParams } = params ?? {}
+  return { serverId, serverName, toolName, userProvidedParams }
+}
+
 export class AgentBlockHandler implements BlockHandler {
   canHandle(block: SerializedBlock): boolean {
     return block.metadata?.id === BlockType.AGENT
@@ -1130,7 +1143,9 @@ export class AgentBlockHandler implements BlockHandler {
     projectedTool?: ToolInput,
     toolIndex?: number
   ): Promise<any> {
-    const { serverId, toolName, serverName, ...userProvidedParams } = tool.params || {}
+    const { serverId, serverName, toolName, userProvidedParams } = splitMcpControlParams(
+      tool.params
+    )
     const projectedSchema = projectedTool?.schema ?? tool.schema
     if (projectedSchema !== undefined && !isPlainRecord(projectedSchema)) {
       refuseResolvedSecretProjection({
@@ -1355,7 +1370,7 @@ export class AgentBlockHandler implements BlockHandler {
     mcpTool: any,
     serverId: string
   ): Promise<any> {
-    const { toolName, ...userProvidedParams } = tool.params || {}
+    const { toolName, userProvidedParams } = splitMcpControlParams(tool.params)
     return this.buildMcpTool({
       serverId,
       toolName,

apps/sim/executor/handlers/pi/local/sim-tools.ts

@@ -233,16 +233,24 @@ export async function buildSimToolSpecs(
   }
 
   const providers = configuredTools.map(({ provider }) => provider)
-  // Pi resolves secret provenance per tool CALL rather than per format, so at this point it cannot
-  // say which individual tool carries one. Withhold literal values for the whole run when any input
-  // resolved a secret — coarse, but it errs toward stating less.
-  const withholdLiteralValues = Boolean(
-    ctx.resolvedSecretTraceRegistry?.hasResolvedInputProjections()
-  )
-  if (withholdLiteralValues) {
-    logger.debug('Withholding pinned literal values: an input in this run resolved a secret')
+
+  // Withhold a tool's literal values only when that tool's own params resolved a secret, asking
+  // the registry the same per-input-path question the Agent block asks. A run-wide flag would be
+  // safe but near-useless here: one `{{API_KEY}}` anywhere in a workflow would blank the literals
+  // on every Pi tool for the whole run.
+  const registry = ctx.resolvedSecretTraceRegistry
+  const withheld = new Set<ProviderToolConfig>()
+  if (registry) {
+    for (const { provider, toolIndex } of configuredTools) {
+      const provenance = registry.exportCommittedProvenanceForInputPaths([
+        ['tools', String(toolIndex), 'params'],
+      ])
+      // An incomplete projection means the registry cannot vouch for the value; treat that the
+      // same as carrying a secret.
+      if (!provenance.complete || provenance.entries.length > 0) withheld.add(provider)
+    }
   }
-  await annotateToolPinnedParams(ctx, providers, () => withholdLiteralValues)
+  await annotateToolPinnedParams(ctx, providers, (tool) => withheld.has(tool))
   assignProviderToolIdentities(providers)
   return configuredTools.map(({ provider, toolIndex }) =>
     buildSimToolSpec(ctx, inputTools, provider, toolIndex)

apps/sim/providers/tool-binding.test.ts

@@ -256,6 +256,51 @@ describe('collectToolPinnedFields', () => {
     expect(fields[0].title).toBe(`${'T'.repeat(40)}…`)
   })
 
+  it('keeps a canonical group blocked when only one half is unstateable', () => {
+    // A `file-upload` basic half beside a `short-input` file-reference twin: the twin must not
+    // state a raw reference (a presigned URL carries its credential) just because its sibling
+    // was skipped.
+    expect(
+      collect({
+        subBlocks: [
+          sub({
+            id: 'attachmentFiles',
+            title: 'Attachments',
+            type: 'file-upload',
+            canonicalParamId: 'attachments',
+          }),
+          sub({
+            id: 'attachments',
+            title: 'Attachments',
+            type: 'short-input',
+            canonicalParamId: 'attachments',
+          }),
+        ],
+        resolvedResourceParams: { attachments: 'https://example.com/f?X-Amz-Signature=abc' },
+        toolParams: toolParams('attachments'),
+      })
+    ).toEqual([])
+  })
+
+  it('keeps a canonical group blocked when only one half is a password field', () => {
+    expect(
+      collect({
+        subBlocks: [
+          sub({
+            id: 'authBasic',
+            title: 'Auth',
+            type: 'short-input',
+            password: true,
+            canonicalParamId: 'auth',
+          }),
+          sub({ id: 'authAdvanced', title: 'Auth', type: 'short-input', canonicalParamId: 'auth' }),
+        ],
+        resolvedResourceParams: { auth: 'hunter2' },
+        toolParams: toolParams('auth'),
+      })
+    ).toEqual([])
+  })
+
   it('drops a non-finite number', () => {
     expect(
       collect({
@@ -275,6 +320,22 @@ describe('collectPinnedFieldsFromParams', () => {
     ])
   })
 
+  it('withholds secret-ish names a remote schema may use', () => {
+    // Param names here are authored by the MCP server, not by Sim, so the Sim-tuned
+    // `isPasswordParameter` list is not sufficient on its own.
+    const remote = {
+      authorization: 'Bearer abc',
+      cookie: 'sid=1',
+      signature: 'deadbeef',
+      connectionString: 'postgres://u:p@h/db',
+      otp: '123456',
+      channel: 'general',
+    }
+    expect(collectPinnedFieldsFromParams(remote, sourceOptions)).toEqual([
+      { title: 'channel', value: 'general' },
+    ])
+  })
+
   it('withholds secrets and unstateable values', () => {
     expect(
       collectPinnedFieldsFromParams(

apps/sim/providers/tool-binding.ts

@@ -52,11 +52,14 @@ const UNSTATEABLE_SUBBLOCK_TYPES: ReadonlySet<string> = new Set([
 ])
 
 /**
- * Covers the one secret spelling `isPasswordParameter` misses — it tests for `password`, not
- * `passphrase`, and three blocks declare a `passphrase` field. Those all set `password: true` as
- * well, so this only matters for a field that forgets the flag.
+ * Secret-ish names `isPasswordParameter` does not cover. It is tuned to Sim-authored param ids
+ * (`password`, `apiKey`, `token`, `secret`, `key`, `credential`, …), but this module also states
+ * params named by a REMOTE MCP schema, where these spellings are common and just as sensitive.
+ * `passphrase` is the one that also occurs in Sim's own blocks — three declare it, all of them
+ * with `password: true`, so that flag is the real guard and this is the backstop.
  */
-const SUPPLEMENTAL_SECRET_PATTERN = /passphrase/i
+const SUPPLEMENTAL_SECRET_PATTERN =
+  /passphrase|authorization|bearer|cookie|session|signature|connectionstring|dsn|webhookurl|\botp\b|\bpin\b/i
 
 /**
  * Shape a configured value must have to be treated as a resolvable resource id.
@@ -186,10 +189,19 @@ export function collectToolPinnedFields(input: CollectToolPinnedFieldsInput): To
   // whole group rather than from whichever subblock is being scanned. Without this a resource
   // entered in advanced mode takes the literal path: a knowledge base id would be stated verbatim,
   // and a credential would be dropped entirely by the secret-name check below.
+  // Decisions that must hold for a whole canonical group, not for whichever half is scanned first.
+  // A group's advanced half is a plain `short-input`, so its kind has to come from the group — and
+  // a group blocked by ANY half must stay blocked, or a `file-upload` basic half would skip without
+  // claiming the param and let its `short-input` twin state a raw file reference.
   const kindByParamId = new Map<string, ResolvableKind>()
+  const blockedParamIds = new Set<string>()
   for (const subBlock of subBlocks) {
+    const paramId = subBlock.canonicalParamId ?? subBlock.id
     const kind = RESOURCE_SUBBLOCK_KINDS[subBlock.type]
-    if (kind) kindByParamId.set(subBlock.canonicalParamId ?? subBlock.id, kind)
+    if (kind) kindByParamId.set(paramId, kind)
+    if (subBlock.password || subBlock.hidden || UNSTATEABLE_SUBBLOCK_TYPES.has(subBlock.type)) {
+      blockedParamIds.add(paramId)
+    }
   }
 
   const fields: ToolPinnedField[] = []
@@ -200,8 +212,7 @@ export function collectToolPinnedFields(input: CollectToolPinnedFieldsInput): To
     if (seenParamIds.has(paramId)) continue
     if (selfDescribedParamId && paramId === selfDescribedParamId) continue
 
-    if (subBlock.password || subBlock.hidden) continue
-    if (UNSTATEABLE_SUBBLOCK_TYPES.has(subBlock.type)) continue
+    if (blockedParamIds.has(paramId)) continue
 
     const kind = kindByParamId.get(paramId)