Skip to content

Commit e92de57

Browse files
authored
fix(executor): seed run-wide access key lists and scope exact-key cache grants (#7850)
- Seed largeValueKeys/fileKeys in createExecutionContext so keys a block records on its per-block context copy reach later blocks; child executors get their own lists, never the parent's - Share one exact-key grant predicate between the async storage check and the sync large-value cache, which previously honored a granted key without checking the key's workspace and workflow - Correct the executionFilesById TSDoc: the index is built per block from block states
1 parent a900c0d commit e92de57

7 files changed

Lines changed: 122 additions & 30 deletions

File tree

apps/sim/executor/execution/executor.test.ts

Lines changed: 46 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* @vitest-environment node
33
*/
44
import { describe, expect, it, vi } from 'vitest'
5+
import { mergeFileKeys, mergeLargeValueKeys } from '@/lib/execution/payloads/access-keys'
56
import { BlockType } from '@/executor/constants'
67
import { DAGBuilder } from '@/executor/dag/builder'
78
import { DAGExecutor } from '@/executor/execution/executor'
@@ -10,6 +11,15 @@ import type { ExecutionContext, ExecutionResult } from '@/executor/types'
1011
import { buildSentinelStartId } from '@/executor/utils/subflow-utils'
1112
import type { SerializedBlock, SerializedWorkflow } from '@/serializer/types'
1213

14+
/** Reaches the executor's private context factory, which every run's root context comes from. */
15+
function createExecutionContext(executor: DAGExecutor, workflowId = 'wf-1'): ExecutionContext {
16+
return (
17+
executor as unknown as {
18+
createExecutionContext: (workflowId: string) => { context: ExecutionContext }
19+
}
20+
).createExecutionContext(workflowId).context
21+
}
22+
1323
function createExecutor(): DAGExecutor {
1424
return new DAGExecutor({
1525
workflow: {
@@ -404,12 +414,7 @@ describe('DAGExecutor createExecutionContext useDraftState', () => {
404414
: ({ useDraftState: opts.metadataUseDraftState } as ExecutionContext['metadata']),
405415
},
406416
})
407-
const { context } = (
408-
executor as unknown as {
409-
createExecutionContext: (workflowId: string) => { context: ExecutionContext }
410-
}
411-
).createExecutionContext('wf-1')
412-
return context.metadata.useDraftState
417+
return createExecutionContext(executor).metadata.useDraftState
413418
}
414419

415420
it('honors explicit useDraftState=true even when isDeployedContext is true (table dispatcher)', () => {
@@ -442,33 +447,21 @@ describe('DAGExecutor executor delegation origin', () => {
442447
contextExtensions: { executorDelegationOrigin },
443448
})
444449

445-
const { context } = (
446-
executor as unknown as {
447-
createExecutionContext: (workflowId: string) => { context: ExecutionContext }
448-
}
449-
).createExecutionContext('child-workflow')
450+
const context = createExecutionContext(executor, 'child-workflow')
450451

451452
expect(context.workflowId).toBe('child-workflow')
452453
expect(context.executorDelegationOrigin).toBe(executorDelegationOrigin)
453454
})
454455
})
455456

456457
describe('DAGExecutor run-scoped permission config cache', () => {
457-
function createContext(executor: DAGExecutor): ExecutionContext {
458-
return (
459-
executor as unknown as {
460-
createExecutionContext: (workflowId: string) => { context: ExecutionContext }
461-
}
462-
).createExecutionContext('wf-1').context
463-
}
464-
465458
it('seeds one cache per run that survives per-block context copies', () => {
466459
const executor = new DAGExecutor({
467460
workflow: { version: '1', blocks: [], connections: [] },
468461
contextExtensions: { workspaceId: 'ws-1' },
469462
})
470463

471-
const context = createContext(executor)
464+
const context = createExecutionContext(executor)
472465
const blockContext = { ...context }
473466

474467
expect(context.permissionConfigCache).toBeInstanceOf(Map)
@@ -477,9 +470,40 @@ describe('DAGExecutor run-scoped permission config cache', () => {
477470

478471
it('never shares the cache between runs', () => {
479472
const workflow = { version: '1', blocks: [], connections: [] }
480-
const parent = createContext(new DAGExecutor({ workflow, contextExtensions: {} }))
481-
const child = createContext(new DAGExecutor({ workflow, contextExtensions: {} }))
473+
const parent = createExecutionContext(new DAGExecutor({ workflow, contextExtensions: {} }))
474+
const child = createExecutionContext(new DAGExecutor({ workflow, contextExtensions: {} }))
482475

483476
expect(child.permissionConfigCache).not.toBe(parent.permissionConfigCache)
484477
})
485478
})
479+
480+
describe('DAGExecutor exact access key lists', () => {
481+
function createContext(contextExtensions: Record<string, unknown>): ExecutionContext {
482+
return createExecutionContext(
483+
new DAGExecutor({
484+
workflow: { version: '1', blocks: [], connections: [] },
485+
contextExtensions,
486+
})
487+
)
488+
}
489+
490+
it('keeps keys a block records on its context copy for later blocks', () => {
491+
const context = createContext({})
492+
493+
mergeLargeValueKeys({ ...context }, ['large-value-key'])
494+
mergeFileKeys({ ...context }, ['file-key'])
495+
496+
expect(context.largeValueKeys).toEqual(['large-value-key'])
497+
expect(context.fileKeys).toEqual(['file-key'])
498+
})
499+
500+
it('shares the lists a run passes in rather than copying them', () => {
501+
const largeValueKeys = ['inherited-large-value-key']
502+
const fileKeys = ['inherited-file-key']
503+
504+
const context = createContext({ largeValueKeys, fileKeys })
505+
506+
expect(context.largeValueKeys).toBe(largeValueKeys)
507+
expect(context.fileKeys).toBe(fileKeys)
508+
})
509+
})

apps/sim/executor/execution/executor.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -418,8 +418,8 @@ export class DAGExecutor {
418418
workspaceId: this.contextExtensions.workspaceId,
419419
executionId: this.contextExtensions.executionId,
420420
largeValueExecutionIds: this.contextExtensions.largeValueExecutionIds,
421-
largeValueKeys: this.contextExtensions.largeValueKeys,
422-
fileKeys: this.contextExtensions.fileKeys,
421+
largeValueKeys: this.contextExtensions.largeValueKeys ?? [],
422+
fileKeys: this.contextExtensions.fileKeys ?? [],
423423
allowLargeValueWorkflowScope: this.contextExtensions.allowLargeValueWorkflowScope,
424424
userId: this.contextExtensions.userId,
425425
principal: this.contextExtensions.principal,

apps/sim/executor/types.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,10 @@ export interface ExecutionContext {
405405
workspaceId?: string
406406
executionId?: string
407407
largeValueExecutionIds?: string[]
408+
/**
409+
* Exact large-value and file keys this run may read. Seeded by the executor so every block's
410+
* shallow context copy appends to one run-wide list; an executor not handed lists starts empty.
411+
*/
408412
largeValueKeys?: string[]
409413
fileKeys?: string[]
410414
allowLargeValueWorkflowScope?: boolean
@@ -447,8 +451,9 @@ export interface ExecutionContext {
447451
* in any block state or workspace row, so nothing else can resolve it. The
448452
* index only *selects*; every read is still authorized on its own.
449453
*
450-
* A Map for the same reason as {@link toolBindingLabelCache}: `blockCtx` is a
451-
* shallow clone per block execution, so only a shared reference survives.
454+
* Built lazily on the block's context from the current block states, so it lives
455+
* for one block: shared by that block's agent turns and tool calls, rebuilt by the
456+
* next block. Files from earlier blocks reach it through their committed outputs.
452457
*/
453458
executionFilesById?: Map<string, UserFile>
454459

apps/sim/lib/execution/payloads/cache.test.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,15 @@ import {
1616
const MB = 1024 * 1024
1717
const SCOPE = { executionId: 'exec-1' }
1818

19-
function makeRef(id: string, size: number): LargeValueRef {
19+
function makeRef(id: string, size: number, overrides: Partial<LargeValueRef> = {}): LargeValueRef {
2020
return {
2121
__simLargeValueRef: true,
2222
version: LARGE_VALUE_REF_VERSION,
2323
id,
2424
kind: 'object',
2525
size,
2626
executionId: 'exec-1',
27+
...overrides,
2728
}
2829
}
2930

@@ -131,3 +132,48 @@ describe('large value cache retention policy', () => {
131132
expect(getLargeValueCacheStats()).toEqual({ entries: 1, trackedBytes: 200 * MB })
132133
})
133134
})
135+
136+
describe('exact-key access to a cached large value', () => {
137+
const key = 'execution/ws-1/wf-1/exec-source/large-value-lv_keyed.json'
138+
const keyedRef = makeRef('lv_keyed', 16, { key, executionId: 'exec-source' })
139+
140+
beforeEach(() => {
141+
clearLargeValueCacheForTests()
142+
cacheLargeValue('lv_keyed', { secret: 'value' }, 16, {
143+
workspaceId: 'ws-1',
144+
workflowId: 'wf-1',
145+
executionId: 'exec-source',
146+
})
147+
})
148+
149+
afterEach(() => {
150+
clearLargeValueCacheForTests()
151+
})
152+
153+
it('serves a key granted to another execution of the same workflow', () => {
154+
expect(
155+
materializeLargeValueRefSync(keyedRef, {
156+
workspaceId: 'ws-1',
157+
workflowId: 'wf-1',
158+
executionId: 'exec-reader',
159+
largeValueKeys: [key],
160+
})
161+
).toEqual({ secret: 'value' })
162+
})
163+
164+
it('refuses a granted key that belongs to another workspace or workflow', () => {
165+
for (const scope of [
166+
{ workspaceId: 'ws-2', workflowId: 'wf-1' },
167+
{ workspaceId: 'ws-1', workflowId: 'wf-2' },
168+
{ workspaceId: undefined, workflowId: undefined },
169+
]) {
170+
expect(
171+
materializeLargeValueRefSync(keyedRef, {
172+
...scope,
173+
executionId: 'exec-reader',
174+
largeValueKeys: [key],
175+
})
176+
).toBeUndefined()
177+
}
178+
})
179+
})

apps/sim/lib/execution/payloads/cache.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {
22
getLargeValueMaterializationError,
3+
isGrantedLargeValueKey,
34
isLargeValueRef,
45
type LargeValueRef,
56
} from '@/lib/execution/payloads/large-value-ref'
@@ -167,7 +168,7 @@ function scopeMatchesRef(
167168
callerScope.executionId,
168169
...(callerScope.largeValueExecutionIds ?? []),
169170
])
170-
if (ref.key && callerScope.largeValueKeys?.includes(ref.key)) {
171+
if (ref.key && isGrantedLargeValueKey(ref.key, callerScope)) {
171172
return true
172173
}
173174
const workflowScopeAllowed =

apps/sim/lib/execution/payloads/large-value-ref.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,22 @@ export interface LargeValueRef {
2020

2121
const LARGE_VALUE_ID_PATTERN = /^lv_[A-Za-z0-9_-]{12}$/
2222

23+
/**
24+
* Whether `key` is an exact large-value grant in `scope`. A grant only reaches values stored under
25+
* the scope's own workspace and workflow, so a key recorded for another tenant never unlocks one.
26+
*/
27+
export function isGrantedLargeValueKey(
28+
key: string,
29+
scope: { workspaceId?: string; workflowId?: string; largeValueKeys?: readonly string[] }
30+
): boolean {
31+
return Boolean(
32+
scope.workspaceId &&
33+
scope.workflowId &&
34+
key.startsWith(`execution/${scope.workspaceId}/${scope.workflowId}/`) &&
35+
scope.largeValueKeys?.includes(key)
36+
)
37+
}
38+
2339
export function isLargeValueStorageKey(key: string, id: string, executionId?: string): boolean {
2440
if (!key.startsWith('execution/')) return false
2541
if (!key.endsWith(`/large-value-${id}.json`)) return false

apps/sim/lib/execution/payloads/materialization.server.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
66
import { isUserFileWithMetadata } from '@/lib/core/utils/user-file'
77
import {
88
getLargeValueMaterializationError,
9+
isGrantedLargeValueKey,
910
isLargeValueRef,
1011
isLargeValueStorageKey,
1112
type LargeValueRef,
@@ -107,7 +108,6 @@ export function assertLargeValueRefAccess(
107108
context.executionId,
108109
...(context.largeValueExecutionIds ?? []),
109110
])
110-
const allowedKeys = new Set(context.largeValueKeys ?? [])
111111

112112
const parts = ref.key?.split('/') ?? []
113113
const [, workspaceId, workflowId, executionId] = parts
@@ -131,7 +131,7 @@ export function assertLargeValueRefAccess(
131131
if (context.workflowId && workflowId !== context.workflowId) {
132132
throw new Error('Large execution value is not available in this execution.')
133133
}
134-
if (allowedKeys.has(ref.key)) {
134+
if (isGrantedLargeValueKey(ref.key, context)) {
135135
return
136136
}
137137
if (ref.executionId && !allowedExecutionIds.has(ref.executionId) && !workflowScopeAllowed) {

0 commit comments

Comments
 (0)