Skip to content

Commit 5c46420

Browse files
icecrasher321claude
andcommitted
fix(audit): exempt a token-response field only where the tool declares it
The reachability audit treated seven credential-derived fields as guaranteed fillers for a required hidden parameter on any OAuth tool. Only `accessToken` is: the resolver assigns it unconditionally. `idToken`, `instanceUrl`, `apiDomain`, `cloudId`, `domain` and `authStyle` are assigned under `if (data.X)` — present on some providers' credentials and absent on others — and `credentialType` additionally only when the tool lists it in `authoritativeParams`. Whether a credential carries one is a fact about the provider that the resolver cannot vouch for. The tool can. `oauth.authoritativeParams` is already the declaration that the token response supplies the named field, and every real case — the eight `microsoft_dynamics_365_*` tools hiding `instanceUrl` — already lists it. So a required hidden parameter in that set is now exempt only when its tool declares it there, generalising the rule `credentialType` alone had. A tool that hides one without declaring it is asserting a filler the resolver may never run, which is the shape this audit exists to reject. Mutation-tested at the real declaration site: stripping `authoritativeParams` from `DYNAMICS_365_OAUTH_CONFIG` fails the audit naming all eight tools with the remedy; restoring passes. Nothing is flagged on the current tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d66b909 commit 5c46420

1 file changed

Lines changed: 33 additions & 20 deletions

File tree

scripts/check-tool-param-reachability.ts

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* `visibility: 'hidden'` means "not shown to user or LLM" (`tools/types.ts`), so
66
* a hidden parameter has no caller. Something else has to supply it, and only
77
* two mechanisms do: OAuth credential resolution, which assigns the fields in
8-
* {@link CREDENTIAL_FILLED} once a credential is bound, and hosted-key
8+
* {@link RESOLVER_GUARANTEED} once a credential is bound, or the fields a tool declares in `authoritativeParams`, and hosted-key
99
* injection, which assigns `hosting.apiKeyParam`. A required hidden parameter
1010
* outside both is unreachable by every caller except the block that happens to
1111
* construct it during serialization.
@@ -54,15 +54,34 @@ import { tools } from '../apps/sim/tools/registry'
5454
import type { ToolConfig } from '../apps/sim/tools/types'
5555

5656
/**
57-
* Parameters `executeToolImplementation` assigns from a resolved credential.
57+
* The one parameter credential resolution assigns unconditionally.
5858
*
59-
* Kept in step with the assignments in `apps/sim/tools/index.ts` — a tool
60-
* declaring `oauth` earns an exemption only for the fields the resolver
61-
* actually writes, so a hidden parameter with an unrelated name is still
62-
* reported even on an OAuth tool.
59+
* `executeToolImplementation` writes `contextParams.accessToken = data.accessToken`
60+
* with no guard, so an OAuth tool's hidden `accessToken` is filled whenever a
61+
* credential resolves at all. Nothing else is: every other token-response field
62+
* is assigned under `if (data.X)`, present on some providers' credentials and
63+
* absent on others.
6364
*/
64-
const CREDENTIAL_FILLED = new Set([
65-
'accessToken',
65+
const RESOLVER_GUARANTEED = 'accessToken'
66+
67+
/**
68+
* Token-response fields a tool may declare its credential supplies.
69+
*
70+
* These are assigned conditionally — `idToken`, `instanceUrl`, `apiDomain`,
71+
* `cloudId`, `domain`, `authStyle` under `if (data.X)`, and `credentialType`
72+
* additionally only when listed here. Whether a given credential carries one
73+
* is a fact about the provider, not the resolver, and the resolver cannot
74+
* vouch for it. The tool can: `oauth.authoritativeParams` is the declaration
75+
* that the token response supplies the named field, so a required hidden
76+
* parameter in this set is exempt only when its tool lists it there. A tool
77+
* that hides one without declaring it is asserting a filler the resolver may
78+
* never run — the exact shape this audit exists to reject.
79+
*
80+
* Kept in step with the assignments in `apps/sim/tools/index.ts` and the
81+
* `authoritativeParams` union in `tools/types.ts`.
82+
*/
83+
const TOKEN_RESPONSE_FIELDS = new Set([
84+
'credentialType',
6685
'idToken',
6786
'instanceUrl',
6887
'apiDomain',
@@ -71,14 +90,6 @@ const CREDENTIAL_FILLED = new Set([
7190
'authStyle',
7291
])
7392

74-
/**
75-
* `credentialType` is the one credential field the resolver gates on more than
76-
* the credential carrying it: it is assigned only when the tool lists it in
77-
* `oauth.authoritativeParams`. Exempting it unconditionally would pass a future
78-
* required-hidden `credentialType` that execution leaves `undefined`.
79-
*/
80-
const AUTHORITATIVE_ONLY = 'credentialType'
81-
8293
interface Finding {
8394
toolId: string
8495
param: string
@@ -120,11 +131,11 @@ function findUnreachableParams(): Finding[] {
120131

121132
for (const [param, declaration] of Object.entries(config.params ?? {})) {
122133
if (!declaration || declaration.visibility !== 'hidden' || !declaration.required) continue
123-
if (config.oauth && CREDENTIAL_FILLED.has(param)) continue
134+
if (config.oauth && param === RESOLVER_GUARANTEED) continue
124135
if (
125136
config.oauth &&
126-
param === AUTHORITATIVE_ONLY &&
127-
config.oauth.authoritativeParams?.includes(AUTHORITATIVE_ONLY)
137+
TOKEN_RESPONSE_FIELDS.has(param) &&
138+
(config.oauth.authoritativeParams as readonly string[] | undefined)?.includes(param)
128139
) {
129140
continue
130141
}
@@ -134,7 +145,9 @@ function findUnreachableParams(): Finding[] {
134145
toolId,
135146
param,
136147
reason: config.oauth
137-
? `declares oauth (${config.oauth.provider}), which does not supply '${param}'`
148+
? TOKEN_RESPONSE_FIELDS.has(param)
149+
? `declares oauth (${config.oauth.provider}) but not \`authoritativeParams: ['${param}']\`, and the resolver assigns '${param}' only when the credential carries it`
150+
: `declares oauth (${config.oauth.provider}), which does not supply '${param}'`
138151
: config.hosting?.enabled
139152
? `hosting is conditional, so it is not a guarantee for '${param}'`
140153
: config.hosting

0 commit comments

Comments
 (0)