Skip to content

Commit e78cdff

Browse files
committed
improvement(execute): reuse the resolved workspace access and dedupe export failures
Route the remaining sandbox-export failure responses through the existing exportFailure helper so the two export helpers build their 400/500 bodies one way instead of two. Body and status are unchanged for every case. Pass the already-resolved workspace access into the workspace file writer from the two copilot callers that were discarding it, so the write no longer re-queries permissions it just resolved.
1 parent 33fe043 commit e78cdff

3 files changed

Lines changed: 55 additions & 98 deletions

File tree

apps/sim/app/api/function/execute/route.ts

Lines changed: 51 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -1453,14 +1453,11 @@ async function maybeExportSandboxFileToWorkspace(args: {
14531453
if (!outputSandboxPath) return null
14541454

14551455
if (!outputPath) {
1456-
return NextResponse.json(
1457-
{
1458-
success: false,
1459-
error:
1460-
'outputSandboxPath requires outputPath. Set outputPath to the destination workspace file, e.g. "files/result.csv".',
1461-
output: { result: null, stdout: cleanStdout(stdout), executionTime },
1462-
},
1463-
{ status: 400 }
1456+
return exportFailure(
1457+
'outputSandboxPath requires outputPath. Set outputPath to the destination workspace file, e.g. "files/result.csv".',
1458+
400,
1459+
stdout,
1460+
executionTime
14641461
)
14651462
}
14661463

@@ -1480,13 +1477,11 @@ async function maybeExportSandboxFileToWorkspace(args: {
14801477
if (!access) return exportFailure('Workspace access denied', 403, stdout, executionTime)
14811478

14821479
if (exportedFileContent === undefined) {
1483-
return NextResponse.json(
1484-
{
1485-
success: false,
1486-
error: `Sandbox file "${outputSandboxPath}" was not found or could not be read`,
1487-
output: { result: null, stdout: cleanStdout(stdout), executionTime },
1488-
},
1489-
{ status: 500 }
1480+
return exportFailure(
1481+
`Sandbox file "${outputSandboxPath}" was not found or could not be read`,
1482+
500,
1483+
stdout,
1484+
executionTime
14901485
)
14911486
}
14921487

@@ -1500,13 +1495,11 @@ async function maybeExportSandboxFileToWorkspace(args: {
15001495
const isBinary = !TEXT_MIMES.has(resolvedMimeType)
15011496
const outputBytes = Buffer.byteLength(exportedFileContent, isBinary ? 'base64' : 'utf-8')
15021497
if (outputBytes > MAX_SANDBOX_OUTPUT_BYTES) {
1503-
return NextResponse.json(
1504-
{
1505-
success: false,
1506-
error: `Sandbox output files exceed ${MAX_SANDBOX_OUTPUT_BYTES} bytes total`,
1507-
output: { result: null, stdout: cleanStdout(stdout), executionTime },
1508-
},
1509-
{ status: 400 }
1498+
return exportFailure(
1499+
`Sandbox output files exceed ${MAX_SANDBOX_OUTPUT_BYTES} bytes total`,
1500+
400,
1501+
stdout,
1502+
executionTime
15101503
)
15111504
}
15121505
const fileBuffer = isBinary
@@ -1579,13 +1572,11 @@ async function maybeExportSandboxFileToWorkspace(args: {
15791572
resources: [{ type: 'file', id: written.id, title: written.name, path: written.vfsPath }],
15801573
})
15811574
} catch (error) {
1582-
return NextResponse.json(
1583-
{
1584-
success: false,
1585-
error: getErrorMessage(error, 'Failed to export sandbox file'),
1586-
output: { result: null, stdout: cleanStdout(stdout), executionTime },
1587-
},
1588-
{ status: 400 }
1575+
return exportFailure(
1576+
getErrorMessage(error, 'Failed to export sandbox file'),
1577+
400,
1578+
stdout,
1579+
executionTime
15891580
)
15901581
}
15911582
}
@@ -1605,17 +1596,11 @@ async function maybeExportSandboxFilesToWorkspace(args: {
16051596
const sandboxFiles = args.outputFiles.filter((file) => file.sandboxPath)
16061597
if (sandboxFiles.length === 0) return null
16071598
if (sandboxFiles.length > MAX_SANDBOX_OUTPUT_FILES) {
1608-
return NextResponse.json(
1609-
{
1610-
success: false,
1611-
error: `Too many sandbox output files requested (${sandboxFiles.length}). Maximum is ${MAX_SANDBOX_OUTPUT_FILES}.`,
1612-
output: {
1613-
result: null,
1614-
stdout: cleanStdout(args.stdout),
1615-
executionTime: args.executionTime,
1616-
},
1617-
},
1618-
{ status: 400 }
1599+
return exportFailure(
1600+
`Too many sandbox output files requested (${sandboxFiles.length}). Maximum is ${MAX_SANDBOX_OUTPUT_FILES}.`,
1601+
400,
1602+
args.stdout,
1603+
args.executionTime
16191604
)
16201605
}
16211606

@@ -1667,17 +1652,11 @@ async function maybeExportSandboxFilesToWorkspace(args: {
16671652
const sandboxPath = file.sandboxPath!
16681653
const content = args.exportedFiles?.[sandboxPath]
16691654
if (content === undefined) {
1670-
return NextResponse.json(
1671-
{
1672-
success: false,
1673-
error: `Sandbox file "${sandboxPath}" was not found or could not be read`,
1674-
output: {
1675-
result: null,
1676-
stdout: cleanStdout(args.stdout),
1677-
executionTime: args.executionTime,
1678-
},
1679-
},
1680-
{ status: 500 }
1655+
return exportFailure(
1656+
`Sandbox file "${sandboxPath}" was not found or could not be read`,
1657+
500,
1658+
args.stdout,
1659+
args.executionTime
16811660
)
16821661
}
16831662
const outputPath = file.formatPath ?? file.path
@@ -1690,17 +1669,11 @@ async function maybeExportSandboxFilesToWorkspace(args: {
16901669
const size = Buffer.byteLength(content, isBinary ? 'base64' : 'utf-8')
16911670
totalOutputBytes += size
16921671
if (totalOutputBytes > MAX_SANDBOX_OUTPUT_BYTES) {
1693-
return NextResponse.json(
1694-
{
1695-
success: false,
1696-
error: `Sandbox output files exceed ${MAX_SANDBOX_OUTPUT_BYTES} bytes total`,
1697-
output: {
1698-
result: null,
1699-
stdout: cleanStdout(args.stdout),
1700-
executionTime: args.executionTime,
1701-
},
1702-
},
1703-
{ status: 400 }
1672+
return exportFailure(
1673+
`Sandbox output files exceed ${MAX_SANDBOX_OUTPUT_BYTES} bytes total`,
1674+
400,
1675+
args.stdout,
1676+
args.executionTime
17041677
)
17051678
}
17061679
const scanBuffer = isBinary ? Buffer.from(content, 'base64') : Buffer.from(content, 'utf-8')
@@ -1740,34 +1713,22 @@ async function maybeExportSandboxFilesToWorkspace(args: {
17401713
)
17411714
validationPaths = validations.map((validation) => validation.vfsPath)
17421715
} catch (error) {
1743-
return NextResponse.json(
1744-
{
1745-
success: false,
1746-
error: getErrorMessage(error, 'Invalid sandbox output destination'),
1747-
output: {
1748-
result: null,
1749-
stdout: cleanStdout(args.stdout),
1750-
executionTime: args.executionTime,
1751-
},
1752-
},
1753-
{ status: 400 }
1716+
return exportFailure(
1717+
getErrorMessage(error, 'Invalid sandbox output destination'),
1718+
400,
1719+
args.stdout,
1720+
args.executionTime
17541721
)
17551722
}
17561723
const duplicateDestination = validationPaths.find(
17571724
(vfsPath, index) => validationPaths.indexOf(vfsPath) !== index
17581725
)
17591726
if (duplicateDestination) {
1760-
return NextResponse.json(
1761-
{
1762-
success: false,
1763-
error: `Duplicate sandbox output destination: ${duplicateDestination}`,
1764-
output: {
1765-
result: null,
1766-
stdout: cleanStdout(args.stdout),
1767-
executionTime: args.executionTime,
1768-
},
1769-
},
1770-
{ status: 400 }
1727+
return exportFailure(
1728+
`Duplicate sandbox output destination: ${duplicateDestination}`,
1729+
400,
1730+
args.stdout,
1731+
args.executionTime
17711732
)
17721733
}
17731734

@@ -1815,17 +1776,11 @@ async function maybeExportSandboxFilesToWorkspace(args: {
18151776
})
18161777
}
18171778
} catch (error) {
1818-
return NextResponse.json(
1819-
{
1820-
success: false,
1821-
error: getErrorMessage(error, 'Failed to export sandbox files'),
1822-
output: {
1823-
result: null,
1824-
stdout: cleanStdout(args.stdout),
1825-
executionTime: args.executionTime,
1826-
},
1827-
},
1828-
{ status: 400 }
1779+
return exportFailure(
1780+
getErrorMessage(error, 'Failed to export sandbox files'),
1781+
400,
1782+
args.stdout,
1783+
args.executionTime
18291784
)
18301785
}
18311786

@@ -1991,7 +1946,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
19911946
const workspaceAccess = workspaceId
19921947
? await checkWorkspaceAccess(workspaceId, auth.userId)
19931948
: undefined
1994-
if (workspaceAccess && !workspaceAccess.hasAccess) {
1949+
if (workspaceAccess && (!workspaceAccess.exists || !workspaceAccess.hasAccess)) {
19951950
logger.warn(`[${requestId}] Function execution denied for workspace`, {
19961951
workspaceId,
19971952
userId: auth.userId,

apps/sim/lib/copilot/tools/server/files/create-file.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export const createFileServerTool: BaseServerTool<CreateFileArgs, CreateFileResu
3939
if (!workspaceId) {
4040
return { success: false, message: 'Workspace ID is required' }
4141
}
42-
await ensureWorkspaceAccess(workspaceId, context.userId, 'write')
42+
const workspaceAccess = await ensureWorkspaceAccess(workspaceId, context.userId, 'write')
4343

4444
const nested = params.args
4545
const fileName = params.fileName || (nested?.fileName as string) || ''
@@ -57,6 +57,7 @@ export const createFileServerTool: BaseServerTool<CreateFileArgs, CreateFileResu
5757
const result = await writeWorkspaceFileByPath({
5858
workspaceId,
5959
userId: context.userId,
60+
workspaceAccess,
6061
target: {
6162
path: outputPath,
6263
mode: outputFile?.mode ?? 'create',

apps/sim/lib/copilot/tools/server/files/download-to-workspace-file.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ export const downloadToWorkspaceFileServerTool: BaseServerTool<
152152
if (!workspaceId) {
153153
return { success: false, message: 'Workspace ID is required' }
154154
}
155-
await ensureWorkspaceAccess(workspaceId, context.userId, 'write')
155+
const workspaceAccess = await ensureWorkspaceAccess(workspaceId, context.userId, 'write')
156156

157157
try {
158158
assertServerToolNotAborted(context)
@@ -192,6 +192,7 @@ export const downloadToWorkspaceFileServerTool: BaseServerTool<
192192
const written = await writeWorkspaceFileByPath({
193193
workspaceId,
194194
userId: context.userId,
195+
workspaceAccess,
195196
target: {
196197
path: outputPath,
197198
mode: outputFile?.mode ?? 'create',

0 commit comments

Comments
 (0)