Skip to content

Commit d9fb301

Browse files
committed
fix(agents): complete direct Function execution and safe diagnostics
1 parent 611bb2e commit d9fb301

10 files changed

Lines changed: 904 additions & 33 deletions

File tree

‎apps/docs/content/docs/workflows/blocks/function.mdx‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,12 +140,19 @@ rather than returning part of what your code wrote.
140140
</Callout>
141141

142142
<Callout type="warn">
143-
Returned files live with the execution rather than in your workspace, and a text
144-
file containing a resolved secret value is refused rather than returned — there is
145-
nowhere on an execution file to record that it carries one. Write such a file to a
146-
workspace path instead, or keep the secret out of the output.
143+
Returned workflow files live with the execution rather than in your workspace.
144+
A file containing a literal protected secret value is refused. Keep secrets out of
145+
returned files, or use an explicit workspace export that records their provenance.
147146
</Callout>
148147

148+
When called directly with `sim tools execute function_execute`, generated files
149+
belong to the authenticated user and have `context: "copilot"`. Download one with
150+
`sim tools files download "<file.id>" --output-file ./result.txt` in the same
151+
workspace context. Direct calls also check filenames and refuse files whose
152+
secret provenance is uncertain, including binary output after a protected secret
153+
was used. For code that needs no secrets, pass `secretScope: "selected"` and
154+
`mountedSecrets: []` in the tool input.
155+
149156
## Language
150157

151158
JavaScript without imports runs in a fast local sandbox. JavaScript with `import` or `require`, Python, and Shell run in the configured remote sandbox provider.

‎apps/sim/lib/environment/utils.test.ts‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,108 @@ describe('getPersonalAndWorkspaceEnv access filtering', () => {
363363
}))
364364
})
365365

366+
it('skips every lookup when no names are selected', async () => {
367+
const snapshot = await getPersonalAndWorkspaceEnv('user-1', 'workspace-1', {
368+
requestedNames: [],
369+
})
370+
expect(snapshot).toEqual({
371+
personalEncrypted: {},
372+
workspaceEncrypted: {},
373+
personalDecrypted: {},
374+
workspaceDecrypted: {},
375+
personalOwners: {},
376+
conflicts: [],
377+
decryptionFailures: [],
378+
workspaceUnredactedKeys: [],
379+
})
380+
expect(mockCheckWorkspaceAccess).not.toHaveBeenCalled()
381+
expect(mockGetAccessibleEnvCredentials).not.toHaveBeenCalled()
382+
expect(encryptionMockFns.mockDecryptSecret).not.toHaveBeenCalled()
383+
})
384+
385+
it('filters selection before decryption while retaining owner, precedence and visibility provenance', async () => {
386+
mockGetAccessibleEnvCredentials.mockResolvedValue([
387+
{
388+
type: 'env_workspace',
389+
envKey: 'SELECTED',
390+
envOwnerUserId: null,
391+
updatedAt: new Date(),
392+
unredacted: true,
393+
},
394+
{
395+
type: 'env_workspace',
396+
envKey: 'OTHER',
397+
envOwnerUserId: null,
398+
updatedAt: new Date(),
399+
unredacted: true,
400+
},
401+
{
402+
type: 'env_personal',
403+
envKey: 'SHARED',
404+
envOwnerUserId: 'owner-2',
405+
updatedAt: new Date(),
406+
unredacted: false,
407+
},
408+
])
409+
queueTableRows(environment, [
410+
{ variables: { SELECTED: 'personal-cipher', OTHER: 'other-personal-cipher' } },
411+
])
412+
queueTableRows(workspaceEnvironment, [
413+
{
414+
variables: {
415+
SELECTED: 'workspace-cipher',
416+
OTHER: 'other-workspace-cipher',
417+
DENIED: 'denied-cipher',
418+
},
419+
},
420+
])
421+
queueTableRows(environment, [
422+
{ userId: 'owner-2', variables: { SHARED: 'shared-cipher', UNRELATED: 'unrelated-cipher' } },
423+
])
424+
const snapshot = await getPersonalAndWorkspaceEnv('user-1', 'workspace-1', {
425+
requestedNames: ['SELECTED', 'SHARED', 'DENIED', 'SELECTED'],
426+
})
427+
expect(snapshot).toMatchObject({
428+
personalEncrypted: { SELECTED: 'personal-cipher', SHARED: 'shared-cipher' },
429+
workspaceEncrypted: { SELECTED: 'workspace-cipher' },
430+
personalDecrypted: { SELECTED: 'plain:personal-cipher', SHARED: 'plain:shared-cipher' },
431+
workspaceDecrypted: { SELECTED: 'plain:workspace-cipher' },
432+
personalOwners: { SELECTED: 'user-1', SHARED: 'owner-2' },
433+
conflicts: ['SELECTED'],
434+
workspaceUnredactedKeys: ['SELECTED'],
435+
})
436+
expect(encryptionMockFns.mockDecryptSecret.mock.calls.flat()).toEqual(
437+
expect.arrayContaining(['personal-cipher', 'workspace-cipher', 'shared-cipher'])
438+
)
439+
expect(encryptionMockFns.mockDecryptSecret).toHaveBeenCalledTimes(3)
440+
})
441+
442+
it('rechecks selected credential access after a grant is revoked', async () => {
443+
const credential = {
444+
type: 'env_workspace',
445+
envKey: 'SELECTED',
446+
envOwnerUserId: null,
447+
updatedAt: new Date(),
448+
unredacted: false,
449+
}
450+
mockGetAccessibleEnvCredentials.mockResolvedValueOnce([credential]).mockResolvedValueOnce([])
451+
for (let index = 0; index < 2; index++) {
452+
queueTableRows(environment, [{ variables: {} }])
453+
queueTableRows(workspaceEnvironment, [{ variables: { SELECTED: 'workspace-cipher' } }])
454+
}
455+
expect(
456+
(await getPersonalAndWorkspaceEnv('user-1', 'workspace-1', { requestedNames: ['SELECTED'] }))
457+
.workspaceDecrypted
458+
).toEqual({ SELECTED: 'plain:workspace-cipher' })
459+
expect(
460+
(await getPersonalAndWorkspaceEnv('user-1', 'workspace-1', { requestedNames: ['SELECTED'] }))
461+
.workspaceDecrypted
462+
).toEqual({})
463+
expect(mockCheckWorkspaceAccess).toHaveBeenCalledTimes(2)
464+
expect(mockGetAccessibleEnvCredentials).toHaveBeenCalledTimes(2)
465+
expect(encryptionMockFns.mockDecryptSecret).toHaveBeenCalledOnce()
466+
})
467+
366468
it('filters every workspace secret when the caller has zero credential grants', async () => {
367469
queueTableRows(environment, [{ variables: { PERSONAL_KEY: 'personal-cipher' } }])
368470
queueTableRows(workspaceEnvironment, [{ variables: { WORKSPACE_KEY: 'workspace-cipher' } }])

‎apps/sim/lib/environment/utils.ts‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -353,10 +353,36 @@ export async function resolveEffectiveEnvironmentVariables(
353353
export async function getPersonalAndWorkspaceEnv(
354354
userId: string,
355355
workspaceId?: string,
356-
options?: { workspaceAccess?: WorkspaceAccess }
356+
options?: { workspaceAccess?: WorkspaceAccess; requestedNames?: readonly string[] }
357357
): Promise<EnvironmentResolutionSnapshot> {
358-
const { personalEncrypted, workspaceEncrypted, personalOwners, workspaceUnredactedKeys } =
359-
await loadAccessibleEncryptedEnvironment(userId, workspaceId, options)
358+
if (options?.requestedNames?.length === 0) {
359+
return {
360+
personalEncrypted: {},
361+
workspaceEncrypted: {},
362+
personalDecrypted: {},
363+
workspaceDecrypted: {},
364+
personalOwners: {},
365+
conflicts: [],
366+
decryptionFailures: [],
367+
workspaceUnredactedKeys: [],
368+
}
369+
}
370+
const accessible = await loadAccessibleEncryptedEnvironment(userId, workspaceId, options)
371+
const requestedNames = options?.requestedNames ? new Set(options.requestedNames) : undefined
372+
const selectRequested = (values: Record<string, string>): Record<string, string> => {
373+
if (!requestedNames) return values
374+
return Object.fromEntries(
375+
[...requestedNames].flatMap((name) =>
376+
Object.hasOwn(values, name) ? [[name, values[name]]] : []
377+
)
378+
)
379+
}
380+
const personalEncrypted = selectRequested(accessible.personalEncrypted)
381+
const workspaceEncrypted = selectRequested(accessible.workspaceEncrypted)
382+
const personalOwners = selectRequested(accessible.personalOwners)
383+
const workspaceUnredactedKeys = accessible.workspaceUnredactedKeys.filter(
384+
(name) => !requestedNames || requestedNames.has(name)
385+
)
360386

361387
const decryptionFailures: string[] = []
362388

‎apps/sim/lib/function-execution/execute-request.ts‎

Lines changed: 83 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ import {
9999
validateWorkspaceFileWriteTarget,
100100
writeWorkspaceFileByPath,
101101
} from '@/lib/mothership/vfs/resource-writer'
102+
import { uploadCopilotFile } from '@/lib/uploads/contexts/copilot'
102103
import { uploadExecutionFile } from '@/lib/uploads/contexts/execution/execution-file-manager'
103104
import {
104105
createWorkspaceFileSecretProvenanceFromRegistry,
@@ -110,6 +111,7 @@ import {
110111
type WorkspaceFileSecretProvenanceIdentity,
111112
} from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance'
112113
import { deleteFiles } from '@/lib/uploads/core/storage-service'
114+
import { deleteFileMetadata } from '@/lib/uploads/server/metadata'
113115
import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils'
114116
import { getWorkflowById } from '@/lib/workflows/utils'
115117
import { rebindWorkspaceFileDelegatedPrincipal } from '@/lib/workspace-files/application/delegated-principal'
@@ -2079,13 +2081,25 @@ function collectedFileName(relativePath: string): string {
20792081
* and the harvest is all-or-nothing by design. Best-effort on purpose: the
20802082
* caller needs to hear why its export was refused, not that the tidy-up failed.
20812083
*/
2082-
async function discardUploadedExecutionFiles(files: readonly UserFile[]): Promise<void> {
2084+
async function discardUploadedSandboxFiles(files: readonly UserFile[]): Promise<void> {
20832085
if (files.length === 0) return
20842086
try {
2085-
await deleteFiles(
2087+
const context = files[0].context === 'copilot' ? 'copilot' : 'execution'
2088+
const result = await deleteFiles(
20862089
files.map((file) => file.key),
2087-
'execution'
2090+
context
20882091
)
2092+
if (context === 'copilot') {
2093+
const failedKeys = new Set(result.failed.map((failure) => failure.key))
2094+
for (const file of files) {
2095+
if (!failedKeys.has(file.key)) await deleteFileMetadata(file.key)
2096+
}
2097+
}
2098+
if (result.failed.length > 0) {
2099+
logger.warn('Could not remove some partially uploaded sandbox output files', {
2100+
fileCount: result.failed.length,
2101+
})
2102+
}
20892103
} catch (error) {
20902104
logger.warn('Could not remove partially uploaded sandbox output files', {
20912105
fileCount: files.length,
@@ -2132,10 +2146,27 @@ async function collectSandboxOutputFiles(args: {
21322146
const resolvedWorkspaceId =
21332147
args.workspaceId ||
21342148
(args.workflowId ? (await getWorkflowById(args.workflowId))?.workspaceId : undefined)
2149+
const personalOwnerId =
2150+
(routeContext.principal.kind === 'session' ||
2151+
routeContext.principal.kind === 'personal_api_key' ||
2152+
routeContext.principal.kind === 'oauth_access_token') &&
2153+
!args.workflowId &&
2154+
!args.executionId
2155+
? routeContext.fileAccessUserId
2156+
: undefined
2157+
const storageTarget = personalOwnerId
2158+
? { context: 'copilot' as const, userId: personalOwnerId }
2159+
: args.workflowId && args.executionId
2160+
? {
2161+
context: 'execution' as const,
2162+
workflowId: args.workflowId,
2163+
executionId: args.executionId,
2164+
}
2165+
: undefined
21352166

21362167
// Fails rather than returning an empty list: the code did produce files, and
21372168
// reporting success without them would read as "your script wrote nothing".
2138-
if (!resolvedWorkspaceId || !args.workflowId || !args.executionId) {
2169+
if (!resolvedWorkspaceId || !storageTarget) {
21392170
return {
21402171
response: exportFailure(
21412172
'Workspace, workflow, and execution context are required to return files from the sandbox.',
@@ -2158,15 +2189,24 @@ async function collectSandboxOutputFiles(args: {
21582189
const mimeType = getMimeTypeFromExtension(getFileExtension(name))
21592190

21602191
/** Literal secrets must be refused regardless of the export's name or encoding. */
2161-
const scannedProvenance = await getOutputFileSecretProvenance(buffer, false, routeContext, {
2192+
let scannedProvenance = await getOutputFileSecretProvenance(buffer, false, routeContext, {
21622193
userId: args.authUserId,
21632194
workspaceId: resolvedWorkspaceId,
21642195
})
2196+
if (personalOwnerId) {
2197+
scannedProvenance = mergeWorkspaceFileSecretProvenance(
2198+
scannedProvenance,
2199+
await getOutputFileSecretProvenance(Buffer.from(name), false, routeContext, {
2200+
userId: personalOwnerId,
2201+
workspaceId: resolvedWorkspaceId,
2202+
})
2203+
)
2204+
}
21652205
if (
21662206
scannedProvenance.status === 'unknown' ||
21672207
(scannedProvenance.status === 'exact' && scannedProvenance.entries.length > 0)
21682208
) {
2169-
await discardUploadedExecutionFiles(files)
2209+
await discardUploadedSandboxFiles(files)
21702210
return {
21712211
response: exportFailure(
21722212
`Sandbox output file "${name}" contains a resolved secret value and was not returned. Write the file without embedding secret values, or export it to a workspace file where its provenance can be recorded.`,
@@ -2192,22 +2232,46 @@ async function collectSandboxOutputFiles(args: {
21922232
})
21932233
: scannedProvenance
21942234

2195-
const userFile = await uploadExecutionFile(
2196-
{
2197-
workspaceId: resolvedWorkspaceId,
2198-
workflowId: args.workflowId,
2199-
executionId: args.executionId,
2200-
},
2201-
buffer,
2202-
name,
2203-
mimeType,
2204-
args.authUserId,
2205-
secretProvenance
2206-
)
2235+
if (
2236+
personalOwnerId &&
2237+
(secretProvenance.status !== 'exact' || secretProvenance.entries.length > 0)
2238+
) {
2239+
await discardUploadedSandboxFiles(files)
2240+
return {
2241+
response: exportFailure(
2242+
`Sandbox output file "${name}" cannot be returned because its secret provenance is uncertain. Return a text file without secret values, or export it from a workflow that records file provenance.`,
2243+
400,
2244+
args.stdout,
2245+
args.executionTime,
2246+
args.cost
2247+
),
2248+
}
2249+
}
2250+
2251+
const userFile =
2252+
storageTarget.context === 'copilot'
2253+
? await uploadCopilotFile({
2254+
buffer,
2255+
fileName: name,
2256+
contentType: mimeType,
2257+
userId: storageTarget.userId,
2258+
})
2259+
: await uploadExecutionFile(
2260+
{
2261+
workspaceId: resolvedWorkspaceId,
2262+
workflowId: storageTarget.workflowId,
2263+
executionId: storageTarget.executionId,
2264+
},
2265+
buffer,
2266+
name,
2267+
mimeType,
2268+
args.authUserId,
2269+
secretProvenance
2270+
)
22072271
files.push(userFile)
22082272
}
22092273
} catch (error) {
2210-
await discardUploadedExecutionFiles(files)
2274+
await discardUploadedSandboxFiles(files)
22112275
throw error
22122276
}
22132277

0 commit comments

Comments
 (0)