Skip to content

Commit b8aff28

Browse files
committed
fix(tools): exempt variable references that resolve to the caller's own key
1 parent 45c027c commit b8aff28

2 files changed

Lines changed: 34 additions & 3 deletions

File tree

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
customBlockOperationsMockFns,
2727
} from '@sim/testing/mocks/custom-block-operations.mock'
2828
import { resetEnvFlagsMock, setEnvFlags } from '@sim/testing/mocks/env-flags.mock'
29+
import { environmentUtilsMockFns } from '@sim/testing/mocks/environment-utils.mock'
2930
import {
3031
integrationsAvailabilityMock,
3132
integrationsAvailabilityMockFns,
@@ -253,6 +254,7 @@ describe('executeToolForCaller', () => {
253254
mocks.executeRegistryTool.mockResolvedValue({ success: true, output: { markdown: '# Hi' } })
254255
mocks.resolveBillingAttribution.mockResolvedValue({ workspaceId: WORKSPACE_ID })
255256
mocks.checkUsageLimits.mockResolvedValue({ isExceeded: false })
257+
environmentUtilsMockFns.mockGetEffectiveDecryptedEnv.mockResolvedValue({})
256258
})
257259

258260
it.each<PersonalApiKeyPrincipal | SessionPrincipal>([principal, createSessionPrincipal()])(
@@ -512,23 +514,31 @@ describe('executeToolForCaller', () => {
512514
it.each([
513515
['the key is omitted', { input: { url: 'https://a.co' } }],
514516
[
515-
'the key is a variable reference that may resolve empty',
517+
'the key references an empty variable',
516518
{ input: { url: 'https://a.co', apiKey: '{{FIRECRAWL_KEY}}' } },
517519
],
518520
])('refuses a hosted-key call over the usage limit when %s', async (_case, input) => {
519521
mocks.checkUsageLimits.mockResolvedValue({ isExceeded: true, message: 'Usage limit exceeded' })
522+
environmentUtilsMockFns.mockGetEffectiveDecryptedEnv.mockResolvedValue({ FIRECRAWL_KEY: ' ' })
520523

521524
await expect(run(input)).rejects.toBeInstanceOf(ToolUsageLimitExceededError)
522525
})
523526

524527
it.each([
525528
['the caller brings their own key', { input: { url: 'https://a.co', apiKey: 'sk-own' } }],
529+
[
530+
'the caller references a variable holding their own key',
531+
{ input: { url: 'https://a.co', apiKey: '{{FIRECRAWL_KEY}}' } },
532+
],
526533
[
527534
'the tool has no hosted key',
528535
{ toolId: 'zendesk_get_ticket', input: { ticketId: '4', subdomain: 'a', apiToken: 't' } },
529536
],
530537
])('does not gate on usage when %s', async (_case, input) => {
531538
mocks.checkUsageLimits.mockResolvedValue({ isExceeded: true, message: 'Usage limit exceeded' })
539+
environmentUtilsMockFns.mockGetEffectiveDecryptedEnv.mockResolvedValue({
540+
FIRECRAWL_KEY: 'fc-own',
541+
})
532542

533543
await expect(run(input)).resolves.toMatchObject({ status: 'succeeded' })
534544
})

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

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,10 @@ import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application'
1717
import { ForbiddenOperationError } from '@/lib/core/application/forbidden'
1818
import { isHosted } from '@/lib/core/config/env-flags'
1919
import { OrchestrationError } from '@/lib/core/orchestration/types'
20+
import { getEffectiveDecryptedEnv } from '@/lib/environment/utils'
2021
import { principalUserId } from '@/lib/integrations/principal-scope.server'
2122
import { toolExecutionOperations } from '@/lib/tool-execution/application/operations'
22-
import { isEnvVarReference } from '@/executor/constants'
23+
import { extractEnvVarName, isEnvVarReference } from '@/executor/constants'
2324
import { executeTool as executeRegistryTool } from '@/tools'
2425
import type { ExecutableToolConfig } from '@/tools/types'
2526
import { getTool } from '@/tools/utils'
@@ -78,6 +79,23 @@ function hostedKeyParamFor(
7879
return tool.hosting.apiKeyParam
7980
}
8081

82+
/**
83+
* Whether a `{{VAR}}` key resolves to a key of the caller's own.
84+
*
85+
* The registry resolves the reference from this same environment before it
86+
* decides on Sim's key, and a variable that is missing or empty leaves the
87+
* parameter for Sim's key to fill.
88+
*/
89+
async function referencesOwnKey(
90+
value: unknown,
91+
userId: string,
92+
workspaceId: string
93+
): Promise<boolean> {
94+
if (typeof value !== 'string' || !isEnvVarReference(value)) return false
95+
const env = await getEffectiveDecryptedEnv(userId, workspaceId)
96+
return Boolean(env[extractEnvVarName(value)]?.trim())
97+
}
98+
8199
/**
82100
* The three spellings the executor accepts for "which credential".
83101
*
@@ -320,7 +338,10 @@ export const executeToolForCaller = defineAuthorizedWorkspaceUseCase({
320338
* has charged it. A BYOK workspace is gated too, since only the registry can
321339
* see that key — the same standing every workflow run is held to.
322340
*/
323-
if (hostedKeyParam) {
341+
if (
342+
hostedKeyParam &&
343+
!(await referencesOwnKey(callerParams[hostedKeyParam], userId, context.workspaceId))
344+
) {
324345
const usage = await checkExecutionUsageLimits(billingAttribution)
325346
if (usage.isExceeded) {
326347
throw new ToolUsageLimitExceededError(

0 commit comments

Comments
 (0)