Skip to content

Commit 4ebefe9

Browse files
committed
fix(tools): bind schema-enrichment reads to executor delegations
Two schema-enrichment callers still sent the deprecated legacy internal JWT to routes that moved onto delegation-only auth, so both 401'd and swallowed the failure: - tools/params.ts fetched a child workflow's input fields with an unsubjected buildAuthHeaders(), leaving the Agent block's workflow_executor inputMapping untyped so the model guessed the child's field names. - tools/schema-enrichers.ts fetched KB tag definitions the same way, which dropped the tags/tagFilters parameter from the knowledge tools entirely. Extract the executionId binding rule into executionScopeForTarget so the three enrichment call sites share one definition.
1 parent 380aca2 commit 4ebefe9

7 files changed

Lines changed: 243 additions & 23 deletions

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { executionScopeForTarget } from '@/executor/utils/delegation'
6+
7+
describe('executionScopeForTarget', () => {
8+
it('binds the execution when the target is the running workflow', () => {
9+
expect(
10+
executionScopeForTarget({ workflowId: 'workflow-1', executionId: 'run-1' }, 'workflow-1')
11+
).toEqual({ executionId: 'run-1' })
12+
})
13+
14+
it('omits the execution for a child workflow, which binds on its own id', () => {
15+
expect(
16+
executionScopeForTarget({ workflowId: 'parent', executionId: 'run-1' }, 'child')
17+
).toEqual({})
18+
})
19+
20+
it('omits the execution outside an active run', () => {
21+
expect(executionScopeForTarget({ workflowId: 'workflow-1' }, 'workflow-1')).toEqual({})
22+
})
23+
24+
it('omits the execution when the context has no workflow to compare', () => {
25+
expect(executionScopeForTarget({ executionId: 'run-1' }, 'workflow-1')).toEqual({})
26+
})
27+
})
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import type { GenerateInternalDelegationTokenInput } from '@/lib/auth/internal'
2+
3+
/**
4+
* Binds the running execution to a delegation only when it targets the workflow that
5+
* is actually running.
6+
*
7+
* A child workflow is a separate resource and binds on its own id, so forwarding the
8+
* parent's `executionId` would assert a run that does not cover the target and the
9+
* delegation would fail to bind. Callers spread the result into their delegation input.
10+
*
11+
* Kept free of runtime imports so client-reachable modules can read it without pulling
12+
* in the executor graph.
13+
*/
14+
export function executionScopeForTarget(
15+
context: { workflowId?: string; executionId?: string },
16+
targetWorkflowId: string
17+
): Pick<GenerateInternalDelegationTokenInput, 'executionId'> {
18+
return context.workflowId === targetWorkflowId && context.executionId
19+
? { executionId: context.executionId }
20+
: {}
21+
}

apps/sim/providers/utils.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,12 @@ async function fetchWorkflowMetadata(
9090
throw new Error('Workflow metadata enrichment requires a trusted execution subject')
9191
}
9292
const { buildAPIUrl, buildExecutorDelegationHeaders } = await import('@/executor/utils/http')
93+
const { executionScopeForTarget } = await import('@/executor/utils/delegation')
9394

9495
const headers = await buildExecutorDelegationHeaders({
9596
subjectUserId: executionContext.userId,
9697
workflowId,
97-
...(executionContext.workflowId === workflowId && executionContext.executionId
98-
? { executionId: executionContext.executionId }
99-
: {}),
98+
...executionScopeForTarget(executionContext, workflowId),
10099
})
101100
const url = buildAPIUrl(`/api/workflows/${workflowId}`)
102101

apps/sim/tools/params.test.ts

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { afterAll, describe, expect, it, vi } from 'vitest'
1+
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
22
import { mergeToolParameters } from '@/tools/merge-params'
33
import * as toolMetadata from '@/tools/metadata'
44
import {
@@ -18,6 +18,17 @@ import {
1818
} from '@/tools/params'
1919
import type { HttpMethod, ParameterVisibility } from '@/tools/types'
2020

21+
const { mockBuildExecutorDelegationHeaders } = vi.hoisted(() => ({
22+
mockBuildExecutorDelegationHeaders: vi
23+
.fn()
24+
.mockResolvedValue({ Authorization: 'Bearer delegation-token' }),
25+
}))
26+
27+
vi.mock('@/executor/utils/http', () => ({
28+
buildExecutorDelegationHeaders: mockBuildExecutorDelegationHeaders,
29+
buildAPIUrl: (path: string) => new URL(path, 'http://localhost:3000'),
30+
}))
31+
2132
const mockToolConfig = {
2233
id: 'test_tool',
2334
name: 'Test Tool',
@@ -648,6 +659,104 @@ describe('Tool Parameters Utils', () => {
648659
})
649660
})
650661

662+
describe('createLLMToolSchema - child workflow input enrichment', () => {
663+
const childWorkflowPayload = {
664+
data: {
665+
state: {
666+
blocks: {
667+
'block-1': {
668+
type: 'starter',
669+
subBlocks: {
670+
inputFormat: {
671+
value: [
672+
{ name: 'email', type: 'string', description: 'Recipient address' },
673+
{ name: 'attempts', type: 'number' },
674+
],
675+
},
676+
},
677+
},
678+
},
679+
},
680+
},
681+
}
682+
683+
const mockFetch = vi.fn()
684+
685+
beforeEach(() => {
686+
mockBuildExecutorDelegationHeaders.mockClear()
687+
mockFetch.mockReset()
688+
mockFetch.mockResolvedValue(
689+
new Response(JSON.stringify(childWorkflowPayload), {
690+
status: 200,
691+
headers: { 'Content-Type': 'application/json' },
692+
})
693+
)
694+
// The suite runs with `unstubGlobals`, which restores globals between tests.
695+
vi.stubGlobal('fetch', mockFetch)
696+
})
697+
698+
it('binds the delegation to the execution subject and the target workflow', async () => {
699+
const { schema } = await createLLMToolSchema(
700+
mockWorkflowExecutorConfig,
701+
{ workflowId: 'child-workflow' },
702+
{
703+
userId: 'user-1',
704+
workflowId: 'parent-workflow',
705+
executionId: 'execution-1',
706+
workspaceId: 'workspace-1',
707+
}
708+
)
709+
710+
expect(mockBuildExecutorDelegationHeaders).toHaveBeenCalledWith({
711+
subjectUserId: 'user-1',
712+
workflowId: 'child-workflow',
713+
})
714+
expect(schema.properties.inputMapping.properties).toEqual({
715+
email: { type: 'string', description: 'Recipient address' },
716+
attempts: { type: 'number', description: 'Input field: attempts' },
717+
})
718+
expect(schema.properties.inputMapping.required).toEqual(['email', 'attempts'])
719+
})
720+
721+
it('carries the executionId when the target is the running workflow', async () => {
722+
await createLLMToolSchema(
723+
mockWorkflowExecutorConfig,
724+
{ workflowId: 'parent-workflow' },
725+
{ userId: 'user-1', workflowId: 'parent-workflow', executionId: 'execution-1' }
726+
)
727+
728+
expect(mockBuildExecutorDelegationHeaders).toHaveBeenCalledWith({
729+
subjectUserId: 'user-1',
730+
workflowId: 'parent-workflow',
731+
executionId: 'execution-1',
732+
})
733+
})
734+
735+
it('leaves inputMapping untyped and issues no request without an execution subject', async () => {
736+
const { schema } = await createLLMToolSchema(
737+
mockWorkflowExecutorConfig,
738+
{ workflowId: 'child-workflow' },
739+
{ workflowId: 'parent-workflow', executionId: 'execution-1' }
740+
)
741+
742+
expect(mockBuildExecutorDelegationHeaders).not.toHaveBeenCalled()
743+
expect(mockFetch).not.toHaveBeenCalled()
744+
expect(schema.properties.inputMapping.properties).toBeUndefined()
745+
})
746+
747+
it('leaves inputMapping untyped when the workflow read is rejected', async () => {
748+
mockFetch.mockResolvedValue(new Response('Unauthorized', { status: 401 }))
749+
750+
const { schema } = await createLLMToolSchema(
751+
mockWorkflowExecutorConfig,
752+
{ workflowId: 'child-workflow' },
753+
{ userId: 'user-1', workflowId: 'parent-workflow' }
754+
)
755+
756+
expect(schema.properties.inputMapping.properties).toBeUndefined()
757+
})
758+
})
759+
651760
describe('mergeToolParameters - inputMapping deep merge', () => {
652761
it.concurrent('should deep merge inputMapping when user provides empty object', () => {
653762
const userProvided = {

apps/sim/tools/params.ts

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -699,7 +699,7 @@ export async function createLLMToolSchema(
699699
if (isWorkflowInputMapping) {
700700
const workflowId = userProvidedParams.workflowId as string
701701
if (workflowId) {
702-
await applyDynamicSchemaForWorkflow(propertySchema, workflowId)
702+
await applyDynamicSchemaForWorkflow(propertySchema, workflowId, enrichmentContext)
703703
}
704704
}
705705

@@ -742,10 +742,11 @@ export async function createLLMToolSchema(
742742
*/
743743
async function applyDynamicSchemaForWorkflow(
744744
propertySchema: SchemaProperty,
745-
workflowId: string
745+
workflowId: string,
746+
context: WorkflowToolExecutionContext
746747
): Promise<void> {
747748
try {
748-
const workflowInputFields = await fetchWorkflowInputFields(workflowId)
749+
const workflowInputFields = await fetchWorkflowInputFields(workflowId, context)
749750

750751
if (workflowInputFields && workflowInputFields.length > 0) {
751752
propertySchema.type = 'object'
@@ -771,19 +772,32 @@ async function applyDynamicSchemaForWorkflow(
771772

772773
/**
773774
* Fetches workflow input fields from the API.
775+
*
776+
* The workflow read route accepts only scoped executor delegations, so the call is
777+
* bound to the acting execution subject.
774778
*/
775779
async function fetchWorkflowInputFields(
776-
workflowId: string
780+
workflowId: string,
781+
context: WorkflowToolExecutionContext
777782
): Promise<Array<{ name: string; type: string; description?: string }>> {
778783
try {
779-
const { buildAuthHeaders, buildAPIUrl } = await import('@/executor/utils/http')
780-
781-
const headers = await buildAuthHeaders()
784+
if (!context.userId) {
785+
throw new Error('Workflow input enrichment requires a trusted execution subject')
786+
}
787+
const { buildAPIUrl, buildExecutorDelegationHeaders } = await import('@/executor/utils/http')
788+
const { executionScopeForTarget } = await import('@/executor/utils/delegation')
789+
790+
const headers = await buildExecutorDelegationHeaders({
791+
subjectUserId: context.userId,
792+
workflowId,
793+
...executionScopeForTarget(context, workflowId),
794+
})
782795
const url = buildAPIUrl(`/api/workflows/${workflowId}`)
783796

784797
const response = await fetch(url.toString(), { headers })
785798
if (!response.ok) {
786-
throw new Error('Failed to fetch workflow')
799+
await response.text().catch(() => {})
800+
throw new Error(`Failed to fetch workflow (${response.status})`)
787801
}
788802

789803
const { data } = await response.json()

apps/sim/tools/schema-enrichers.test.ts

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,12 @@
33
*/
44
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
55

6-
const { mockBuildAPIUrl, mockBuildAuthHeaders, mockExtractAPIErrorMessage } = vi.hoisted(() => ({
6+
const {
7+
mockBuildAPIUrl,
8+
mockBuildAuthHeaders,
9+
mockBuildExecutorDelegationHeaders,
10+
mockExtractAPIErrorMessage,
11+
} = vi.hoisted(() => ({
712
mockBuildAPIUrl: vi.fn((path: string, params?: Record<string, string>) => {
813
const url = new URL(path, 'http://localhost:3000')
914
for (const [key, value] of Object.entries(params ?? {})) {
@@ -12,12 +17,14 @@ const { mockBuildAPIUrl, mockBuildAuthHeaders, mockExtractAPIErrorMessage } = vi
1217
return url
1318
}),
1419
mockBuildAuthHeaders: vi.fn(),
20+
mockBuildExecutorDelegationHeaders: vi.fn(),
1521
mockExtractAPIErrorMessage: vi.fn(),
1622
}))
1723

1824
vi.mock('@/executor/utils/http', () => ({
1925
buildAPIUrl: mockBuildAPIUrl,
2026
buildAuthHeaders: mockBuildAuthHeaders,
27+
buildExecutorDelegationHeaders: mockBuildExecutorDelegationHeaders,
2128
extractAPIErrorMessage: mockExtractAPIErrorMessage,
2229
}))
2330

@@ -106,14 +113,16 @@ describe('enrichTableToolSchema', () => {
106113
describe('enrichKBTagsSchema', () => {
107114
beforeEach(() => {
108115
vi.clearAllMocks()
109-
mockBuildAuthHeaders.mockResolvedValue({ Authorization: 'Bearer internal-token' })
116+
mockBuildExecutorDelegationHeaders.mockResolvedValue({
117+
Authorization: 'Bearer delegation-token',
118+
})
110119
})
111120

112121
afterEach(() => {
113122
vi.unstubAllGlobals()
114123
})
115124

116-
it('fetches tag definitions as the acting user so the route can authorize them', async () => {
125+
it('binds the tag-definition read to the acting subject and workflow execution', async () => {
117126
const mockFetch = vi.fn().mockResolvedValue(
118127
new Response(
119128
JSON.stringify({
@@ -125,18 +134,46 @@ describe('enrichKBTagsSchema', () => {
125134
)
126135
vi.stubGlobal('fetch', mockFetch)
127136

128-
const result = await enrichKBTagsSchema('kb-1', { userId: 'user-1' })
137+
const result = await enrichKBTagsSchema('kb-1', {
138+
userId: 'user-1',
139+
workflowId: 'workflow-1',
140+
executionId: 'execution-1',
141+
})
129142

130-
expect(mockBuildAuthHeaders).toHaveBeenCalledWith('user-1')
143+
expect(mockBuildExecutorDelegationHeaders).toHaveBeenCalledWith({
144+
subjectUserId: 'user-1',
145+
workflowId: 'workflow-1',
146+
executionId: 'execution-1',
147+
})
131148
expect(result?.properties).toEqual({ Client: { type: 'string', description: 'text tag' } })
132149
})
133150

134-
it('skips enrichment without an acting user rather than issuing an unauthorized request', async () => {
151+
it('omits the executionId outside an active run', async () => {
152+
const mockFetch = vi.fn().mockResolvedValue(
153+
new Response(JSON.stringify({ success: true, data: [] }), {
154+
status: 200,
155+
headers: { 'Content-Type': 'application/json' },
156+
})
157+
)
158+
vi.stubGlobal('fetch', mockFetch)
159+
160+
await enrichKBTagsSchema('kb-1', { userId: 'user-1', workflowId: 'workflow-1' })
161+
162+
expect(mockBuildExecutorDelegationHeaders).toHaveBeenCalledWith({
163+
subjectUserId: 'user-1',
164+
workflowId: 'workflow-1',
165+
})
166+
})
167+
168+
it.each([
169+
['no acting user', { workflowId: 'workflow-1' }],
170+
['no acting workflow to bind the delegation on', { userId: 'user-1' }],
171+
])('skips enrichment with %s rather than issuing an unauthorized request', async (_, context) => {
135172
const mockFetch = vi.fn()
136173
vi.stubGlobal('fetch', mockFetch)
137174

138-
await expect(enrichKBTagsSchema('kb-1', {})).resolves.toBeNull()
175+
await expect(enrichKBTagsSchema('kb-1', context)).resolves.toBeNull()
139176
expect(mockFetch).not.toHaveBeenCalled()
140-
expect(mockBuildAuthHeaders).not.toHaveBeenCalled()
177+
expect(mockBuildExecutorDelegationHeaders).not.toHaveBeenCalled()
141178
})
142179
})

apps/sim/tools/schema-enrichers.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,10 @@ function mapFieldTypeToSchemaType(fieldType: string): string {
116116
/**
117117
* Fetches tag definitions from a knowledge base as the acting user, whose id the
118118
* route requires to authorize the read.
119+
*
120+
* The tag-definition route accepts only scoped executor delegations. The delegation
121+
* binds on the running workflow — that is what resolves the workspace the knowledge
122+
* base must belong to — so both the subject and the workflow are required.
119123
*/
120124
async function fetchTagDefinitions(
121125
knowledgeBaseId: string,
@@ -125,11 +129,20 @@ async function fetchTagDefinitions(
125129
logger.warn(`Skipping tag definition enrichment for KB ${knowledgeBaseId}: no acting user`)
126130
return []
127131
}
132+
if (!context.workflowId) {
133+
logger.warn(`Skipping tag definition enrichment for KB ${knowledgeBaseId}: no acting workflow`)
134+
return []
135+
}
128136

129137
try {
130-
const { buildAuthHeaders, buildAPIUrl } = await import('@/executor/utils/http')
131-
132-
const headers = await buildAuthHeaders(context.userId)
138+
const { buildAPIUrl, buildExecutorDelegationHeaders } = await import('@/executor/utils/http')
139+
const { executionScopeForTarget } = await import('@/executor/utils/delegation')
140+
141+
const headers = await buildExecutorDelegationHeaders({
142+
subjectUserId: context.userId,
143+
workflowId: context.workflowId,
144+
...executionScopeForTarget(context, context.workflowId),
145+
})
133146
const url = buildAPIUrl(`/api/knowledge/${knowledgeBaseId}/tag-definitions`)
134147

135148
logger.info(`Fetching tag definitions for KB ${knowledgeBaseId} from ${url.toString()}`)

0 commit comments

Comments
 (0)