Skip to content

Commit 54172b7

Browse files
committed
fix(agents): complete direct Function execution and safe diagnostics
1 parent 7f66b15 commit 54172b7

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
@@ -305,6 +305,108 @@ describe('getPersonalAndWorkspaceEnv access filtering', () => {
305305
}))
306306
})
307307

308+
it('skips every lookup when no names are selected', async () => {
309+
const snapshot = await getPersonalAndWorkspaceEnv('user-1', 'workspace-1', {
310+
requestedNames: [],
311+
})
312+
expect(snapshot).toEqual({
313+
personalEncrypted: {},
314+
workspaceEncrypted: {},
315+
personalDecrypted: {},
316+
workspaceDecrypted: {},
317+
personalOwners: {},
318+
conflicts: [],
319+
decryptionFailures: [],
320+
workspaceUnredactedKeys: [],
321+
})
322+
expect(mockCheckWorkspaceAccess).not.toHaveBeenCalled()
323+
expect(mockGetAccessibleEnvCredentials).not.toHaveBeenCalled()
324+
expect(encryptionMockFns.mockDecryptSecret).not.toHaveBeenCalled()
325+
})
326+
327+
it('filters selection before decryption while retaining owner, precedence and visibility provenance', async () => {
328+
mockGetAccessibleEnvCredentials.mockResolvedValue([
329+
{
330+
type: 'env_workspace',
331+
envKey: 'SELECTED',
332+
envOwnerUserId: null,
333+
updatedAt: new Date(),
334+
unredacted: true,
335+
},
336+
{
337+
type: 'env_workspace',
338+
envKey: 'OTHER',
339+
envOwnerUserId: null,
340+
updatedAt: new Date(),
341+
unredacted: true,
342+
},
343+
{
344+
type: 'env_personal',
345+
envKey: 'SHARED',
346+
envOwnerUserId: 'owner-2',
347+
updatedAt: new Date(),
348+
unredacted: false,
349+
},
350+
])
351+
queueTableRows(environment, [
352+
{ variables: { SELECTED: 'personal-cipher', OTHER: 'other-personal-cipher' } },
353+
])
354+
queueTableRows(workspaceEnvironment, [
355+
{
356+
variables: {
357+
SELECTED: 'workspace-cipher',
358+
OTHER: 'other-workspace-cipher',
359+
DENIED: 'denied-cipher',
360+
},
361+
},
362+
])
363+
queueTableRows(environment, [
364+
{ userId: 'owner-2', variables: { SHARED: 'shared-cipher', UNRELATED: 'unrelated-cipher' } },
365+
])
366+
const snapshot = await getPersonalAndWorkspaceEnv('user-1', 'workspace-1', {
367+
requestedNames: ['SELECTED', 'SHARED', 'DENIED', 'SELECTED'],
368+
})
369+
expect(snapshot).toMatchObject({
370+
personalEncrypted: { SELECTED: 'personal-cipher', SHARED: 'shared-cipher' },
371+
workspaceEncrypted: { SELECTED: 'workspace-cipher' },
372+
personalDecrypted: { SELECTED: 'plain:personal-cipher', SHARED: 'plain:shared-cipher' },
373+
workspaceDecrypted: { SELECTED: 'plain:workspace-cipher' },
374+
personalOwners: { SELECTED: 'user-1', SHARED: 'owner-2' },
375+
conflicts: ['SELECTED'],
376+
workspaceUnredactedKeys: ['SELECTED'],
377+
})
378+
expect(encryptionMockFns.mockDecryptSecret.mock.calls.flat()).toEqual(
379+
expect.arrayContaining(['personal-cipher', 'workspace-cipher', 'shared-cipher'])
380+
)
381+
expect(encryptionMockFns.mockDecryptSecret).toHaveBeenCalledTimes(3)
382+
})
383+
384+
it('rechecks selected credential access after a grant is revoked', async () => {
385+
const credential = {
386+
type: 'env_workspace',
387+
envKey: 'SELECTED',
388+
envOwnerUserId: null,
389+
updatedAt: new Date(),
390+
unredacted: false,
391+
}
392+
mockGetAccessibleEnvCredentials.mockResolvedValueOnce([credential]).mockResolvedValueOnce([])
393+
for (let index = 0; index < 2; index++) {
394+
queueTableRows(environment, [{ variables: {} }])
395+
queueTableRows(workspaceEnvironment, [{ variables: { SELECTED: 'workspace-cipher' } }])
396+
}
397+
expect(
398+
(await getPersonalAndWorkspaceEnv('user-1', 'workspace-1', { requestedNames: ['SELECTED'] }))
399+
.workspaceDecrypted
400+
).toEqual({ SELECTED: 'plain:workspace-cipher' })
401+
expect(
402+
(await getPersonalAndWorkspaceEnv('user-1', 'workspace-1', { requestedNames: ['SELECTED'] }))
403+
.workspaceDecrypted
404+
).toEqual({})
405+
expect(mockCheckWorkspaceAccess).toHaveBeenCalledTimes(2)
406+
expect(mockGetAccessibleEnvCredentials).toHaveBeenCalledTimes(2)
407+
expect(encryptionMockFns.mockDecryptSecret).toHaveBeenCalledOnce()
408+
})
409+
308410
it('filters every workspace secret when the caller has zero credential grants', async () => {
309411
queueTableRows(environment, [{ variables: { PERSONAL_KEY: 'personal-cipher' } }])
310412
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)