Skip to content

Commit f156cff

Browse files
icecrasher321claude
andcommitted
fix(tools): refuse a caller-supplied hidden parameter
Declared was being treated as accepted. A `hidden` parameter is Sim's to fill — a resolved credential's `accessToken`, a hosted key, a block-composed shape — and `createUserToolSchema` omits it from what this endpoint and Copilot publish. Accepting it anyway either let a caller pre-empt the executor's value or silently discarded theirs when the executor overwrote it; either way the published schema made no such promise. The accept-set is now exactly the publish-set: a key is taken if and only if `GET /api/v2/tools/{toolId}` lists it as something the caller may send. This is the rule the required-input check already followed ("Sim fills it, or the caller must"), applied to the other direction. Verified live against a dev server: a forged `accessToken` on `gmail_read_v2` is refused before dispatch, and the credential-backed read still succeeds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent aa271a2 commit f156cff

2 files changed

Lines changed: 47 additions & 2 deletions

File tree

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

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,10 @@ const TOOL_METADATA: Record<string, Record<string, unknown>> = {
9999
slack_message: {
100100
id: 'slack_message',
101101
name: 'Slack Send Message',
102-
params: { text: { type: 'string', required: true, visibility: 'user-or-llm' } },
102+
params: {
103+
accessToken: { type: 'string', required: true, visibility: 'hidden' },
104+
text: { type: 'string', required: true, visibility: 'user-or-llm' },
105+
},
103106
oauth: { required: true, provider: 'slack' },
104107
},
105108
firecrawl_scrape: {
@@ -429,6 +432,26 @@ describe('executeToolForCaller', () => {
429432
expect(mocks.executeRegistryTool).not.toHaveBeenCalled()
430433
})
431434

435+
/**
436+
* Declared is not accepted. `accessToken` is in the tool's params, but as
437+
* `hidden` — the resolved credential fills it. Letting a caller send it either
438+
* pre-empts the executor's value or is silently overwritten; either way the
439+
* published schema (which omits hidden params) made no such promise.
440+
*/
441+
it('refuses a declared-but-hidden input, saying whose it is', async () => {
442+
await expect(
443+
run({
444+
toolId: 'slack_message',
445+
credentialId: 'cred-1',
446+
input: { text: 'hi', accessToken: 'xoxb-forged' },
447+
})
448+
).rejects.toMatchObject({
449+
code: 'validation',
450+
message: expect.stringContaining('input.accessToken is supplied by Sim'),
451+
})
452+
expect(mocks.executeRegistryTool).not.toHaveBeenCalled()
453+
})
454+
432455
it('refuses any other undeclared input, naming it', async () => {
433456
await expect(run({ input: { url: 'https://a.co', nope: 1 } })).rejects.toMatchObject({
434457
code: 'validation',

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@ function hostedKeyParamFor(
7777
* parameter list is the actual boundary, and it is what
7878
* `GET /api/v2/tools/{toolId}` already publishes.
7979
*
80+
* Together with the `hidden` refusal above, the accept-set is exactly the
81+
* publish-set: a key is taken if and only if `GET /api/v2/tools/{toolId}`
82+
* lists it as something the caller may send.
83+
*
8084
* Also collapses the credential spellings. The executor accepts `credential`,
8185
* `credentialId` and `oauthCredential` interchangeably, which is fine where one
8286
* caller writes one of them and wrong on a public contract: three spellings with
@@ -88,7 +92,25 @@ function assertNoUndeclaredInputs(
8892
toolId: string,
8993
args: Record<string, unknown>
9094
): void {
91-
const undeclared = Object.keys(args).filter((key) => !Object.hasOwn(tool.params ?? {}, key))
95+
const params = tool.params ?? {}
96+
97+
/**
98+
* Declared is not the same as accepted. A `hidden` parameter is Sim's to fill
99+
* — a resolved credential's `accessToken`, a hosted key, a block-composed
100+
* shape — and `createUserToolSchema` omits it from what this endpoint and
101+
* Copilot publish. Accepting it anyway either lets a caller pre-empt the
102+
* executor's value or silently discards theirs when the executor overwrites
103+
* it, and both are a contract the published schema does not make.
104+
*/
105+
const hidden = Object.keys(args).filter((key) => params[key]?.visibility === 'hidden')
106+
if (hidden.length > 0) {
107+
throw new OrchestrationError(
108+
'validation',
109+
`${hidden.map((key) => `input.${key}`).join(', ')} ${hidden.length === 1 ? 'is' : 'are'} supplied by Sim, not by the caller`
110+
)
111+
}
112+
113+
const undeclared = Object.keys(args).filter((key) => !Object.hasOwn(params, key))
92114
if (undeclared.length === 0) return
93115

94116
const credentialAlias = undeclared.find((key) =>

0 commit comments

Comments
 (0)