Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 120 additions & 0 deletions apps/sim/app/api/function/execute/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,17 @@ import {
SandboxOutputLimitError,
} from '@/lib/execution/remote-sandbox/output-limits'

function grantedAccess(workspaceId: string) {
return {
exists: true,
hasAccess: true,
canWrite: true,
canAdmin: false,
workspace: { id: workspaceId },
permission: 'admin',
}
}

const {
mockExecuteInSandbox,
mockExecuteInIsolatedVM,
Expand All @@ -41,6 +52,8 @@ const {
mockUploadFile,
mockValidateWorkspaceFileWriteTarget,
mockWriteWorkspaceFileByPath,
mockCheckWorkspaceAccess,
mockResolveWorkspaceAccess,
} = vi.hoisted(() => ({
mockExecuteInSandbox: vi.fn(),
mockExecuteInIsolatedVM: vi.fn(),
Expand All @@ -59,6 +72,13 @@ const {
mockUploadFile: vi.fn(),
mockValidateWorkspaceFileWriteTarget: vi.fn(),
mockWriteWorkspaceFileByPath: vi.fn(),
mockCheckWorkspaceAccess: vi.fn(),
mockResolveWorkspaceAccess: vi.fn(),
}))

vi.mock('@/lib/workspaces/permissions/utils', () => ({
checkWorkspaceAccess: mockCheckWorkspaceAccess,
resolveWorkspaceAccess: mockResolveWorkspaceAccess,
}))

vi.mock('@/lib/core/security/encryption', () => ({
Expand Down Expand Up @@ -153,6 +173,9 @@ describe('Function Execute API Route', () => {
authType: 'internal_jwt',
})

mockCheckWorkspaceAccess.mockImplementation(async (id: string) => grantedAccess(id))
mockResolveWorkspaceAccess.mockImplementation(async (id: string) => grantedAccess(id))

mockExecuteInIsolatedVM.mockResolvedValue({ result: 'test', stdout: '' })
mockUploadFile.mockImplementation(async ({ customKey }) => ({ key: customKey }))
clearLargeValueCacheForTests()
Expand Down Expand Up @@ -218,6 +241,103 @@ describe('Function Execute API Route', () => {
expect(data).toHaveProperty('error', 'Unauthorized')
})

it('rejects a body-supplied workspaceId the acting user is not a member of', async () => {
mockCheckWorkspaceAccess.mockResolvedValue({
exists: true,
hasAccess: false,
canWrite: false,
canAdmin: false,
workspace: { id: 'workspace-victim' },
permission: null,
})

const req = createMockRequest('POST', {
code: 'return "test"',
workspaceId: 'workspace-victim',
})

const response = await POST(req)
const data = await response.json()

expect(response.status).toBe(403)
expect(data).toHaveProperty('error', 'Workspace access denied')
expect(mockCheckWorkspaceAccess).toHaveBeenCalledWith('workspace-victim', 'user-123')
expect(mockExecuteInIsolatedVM).not.toHaveBeenCalled()
})

it('rejects a sandbox output export into a workspace the acting user cannot write to', async () => {
envFlagsMock.isRemoteSandboxEnabled = true
mockExecuteInSandbox.mockResolvedValueOnce({
result: 'done',
stdout: 'ok',
sandboxId: 'sandbox-123',
exportedFiles: { '/tmp/out.txt': 'owned by attacker' },
})
const readOnly = {
exists: true,
hasAccess: true,
canWrite: false,
canAdmin: false,
workspace: { id: 'workspace-victim' },
permission: 'read',
}
mockCheckWorkspaceAccess.mockResolvedValue(readOnly)
mockResolveWorkspaceAccess.mockResolvedValue(readOnly)

const req = createMockRequest('POST', {
code: 'print("done")',
language: 'python',
workspaceId: 'workspace-victim',
outputs: {
files: [{ path: 'files/README.md', mode: 'overwrite', sandboxPath: '/tmp/out.txt' }],
},
})

const response = await POST(req)
const data = await response.json()

expect(response.status).toBe(403)
expect(data).toHaveProperty('error', 'Workspace access denied')
expect(mockValidateWorkspaceFileWriteTarget).not.toHaveBeenCalled()
expect(mockWriteWorkspaceFileByPath).not.toHaveBeenCalled()
})

it('rejects an export whose workspace is derived from a body-supplied workflowId', async () => {
envFlagsMock.isRemoteSandboxEnabled = true
mockExecuteInSandbox.mockResolvedValueOnce({
result: 'done',
stdout: 'ok',
sandboxId: 'sandbox-123',
exportedFiles: { '/tmp/out.txt': 'owned by attacker' },
})
workflowsUtilsMock.getWorkflowById.mockResolvedValueOnce({
id: 'workflow-victim',
workspaceId: 'workspace-victim',
})
mockResolveWorkspaceAccess.mockResolvedValue({
exists: true,
hasAccess: false,
canWrite: false,
canAdmin: false,
workspace: { id: 'workspace-victim' },
permission: null,
})

const req = createMockRequest('POST', {
code: 'print("done")',
language: 'python',
workflowId: 'workflow-victim',
outputs: {
files: [{ path: 'files/README.md', mode: 'overwrite', sandboxPath: '/tmp/out.txt' }],
},
})

const response = await POST(req)

expect(response.status).toBe(403)
expect(mockWriteWorkspaceFileByPath).not.toHaveBeenCalled()
})

it('runs import-free JavaScript in isolated-vm without a remote provider', async () => {
const req = createMockRequest('POST', {
code: 'return "test"',
Expand Down
103 changes: 85 additions & 18 deletions apps/sim/app/api/function/execute/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ import {
type WorkspaceFileSecretProvenance,
} from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance'
import { getWorkflowById } from '@/lib/workflows/utils'
import {
checkWorkspaceAccess,
resolveWorkspaceAccess,
type WorkspaceAccess,
} from '@/lib/workspaces/permissions/utils'
import { escapeRegExp, normalizeName, REFERENCE } from '@/executor/constants'
import { type OutputSchema, resolveBlockReference } from '@/executor/utils/block-reference'
import {
Expand Down Expand Up @@ -1383,11 +1388,41 @@ function exportUnchangedNote(sandboxPath?: string): string {
)
}

function exportFailure(
error: string,
status: number,
stdout: string,
executionTime: number
): NextResponse {
return NextResponse.json(
{ success: false, error, output: { result: null, stdout: cleanStdout(stdout), executionTime } },
{ status }
)
}

/**
* Both `workspaceId` and `workflowId` arrive in the request body, so the workspace an export
* resolves to is caller-controlled either way. Returns null when the acting user cannot write to
* it, gating the secret-provenance scan and overwrite probe that run before the write itself.
*/
async function authorizeExportWorkspace(
workspaceId: string,
authUserId: string,
provided?: WorkspaceAccess
): Promise<WorkspaceAccess | null> {
const access = await resolveWorkspaceAccess(workspaceId, authUserId, provided)
if (access.exists && access.canWrite) return access

logger.warn('Sandbox file export denied for workspace', { workspaceId, userId: authUserId })
return null
}

async function maybeExportSandboxFileToWorkspace(args: {
routeContext: FunctionRouteExecutionContext
authUserId: string
workflowId?: string
workspaceId?: string
workspaceAccess?: WorkspaceAccess
outputPath?: string
outputFormat?: string
outputMimeType?: string
Expand All @@ -1403,6 +1438,7 @@ async function maybeExportSandboxFileToWorkspace(args: {
authUserId,
workflowId,
workspaceId,
workspaceAccess,
outputPath,
outputFormat,
outputMimeType,
Expand Down Expand Up @@ -1432,16 +1468,17 @@ async function maybeExportSandboxFileToWorkspace(args: {
workspaceId || (workflowId ? (await getWorkflowById(workflowId))?.workspaceId : undefined)

if (!resolvedWorkspaceId) {
return NextResponse.json(
{
success: false,
error: 'Workspace context required to save sandbox file to workspace',
output: { result: null, stdout: cleanStdout(stdout), executionTime },
},
{ status: 400 }
return exportFailure(
'Workspace context required to save sandbox file to workspace',
400,
stdout,
executionTime
)
}

const access = await authorizeExportWorkspace(resolvedWorkspaceId, authUserId, workspaceAccess)
if (!access) return exportFailure('Workspace access denied', 403, stdout, executionTime)

if (exportedFileContent === undefined) {
return NextResponse.json(
{
Expand Down Expand Up @@ -1496,6 +1533,7 @@ async function maybeExportSandboxFileToWorkspace(args: {
const written = await writeWorkspaceFileByPath({
workspaceId: resolvedWorkspaceId,
userId: authUserId,
workspaceAccess: access,
target: {
path: targetPath,
mode,
Expand Down Expand Up @@ -1557,6 +1595,7 @@ async function maybeExportSandboxFilesToWorkspace(args: {
authUserId: string
workflowId?: string
workspaceId?: string
workspaceAccess?: WorkspaceAccess
outputFiles: OutputFileDeclaration[]
exportedFiles?: Record<string, string>
exportedFileContent?: string
Expand Down Expand Up @@ -1587,6 +1626,7 @@ async function maybeExportSandboxFilesToWorkspace(args: {
authUserId: args.authUserId,
workflowId: args.workflowId,
workspaceId: args.workspaceId,
workspaceAccess: args.workspaceAccess,
outputPath: file.formatPath ?? file.path,
outputFormat: file.format,
outputMimeType: file.mimeType,
Expand All @@ -1604,20 +1644,23 @@ async function maybeExportSandboxFilesToWorkspace(args: {
args.workspaceId ||
(args.workflowId ? (await getWorkflowById(args.workflowId))?.workspaceId : undefined)
if (!resolvedWorkspaceId) {
return NextResponse.json(
{
success: false,
error: 'Workspace context required to save sandbox files to workspace',
output: {
result: null,
stdout: cleanStdout(args.stdout),
executionTime: args.executionTime,
},
},
{ status: 400 }
return exportFailure(
'Workspace context required to save sandbox files to workspace',
400,
args.stdout,
args.executionTime
)
}

const access = await authorizeExportWorkspace(
resolvedWorkspaceId,
args.authUserId,
args.workspaceAccess
)
if (!access) {
return exportFailure('Workspace access denied', 403, args.stdout, args.executionTime)
}

const preparedFiles = []
let totalOutputBytes = 0
for (const file of sandboxFiles) {
Expand Down Expand Up @@ -1690,6 +1733,7 @@ async function maybeExportSandboxFilesToWorkspace(args: {
validateWorkspaceFileWriteTarget({
workspaceId: resolvedWorkspaceId,
userId: args.authUserId,
workspaceAccess: access,
target: prepared.target,
})
)
Expand Down Expand Up @@ -1744,6 +1788,7 @@ async function maybeExportSandboxFilesToWorkspace(args: {
const written = await writeWorkspaceFileByPath({
workspaceId: resolvedWorkspaceId,
userId: args.authUserId,
workspaceAccess: access,
target: prepared.target,
buffer,
inferredMimeType: prepared.resolvedMimeType,
Expand Down Expand Up @@ -1938,6 +1983,25 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
isCustomTool = false,
_sandboxFiles,
} = body

// The internal JWT carries no workspace scope, so a body-supplied workspaceId would
// otherwise be the sole authorization input for sandbox selection and file exports.
// Denial is returned rather than thrown: this handler's catch-all would turn a thrown
// WorkspaceAccessDeniedError into a 500 before withRouteHandler could map it.
const workspaceAccess = workspaceId
? await checkWorkspaceAccess(workspaceId, auth.userId)
: undefined
if (workspaceAccess && !workspaceAccess.hasAccess) {
logger.warn(`[${requestId}] Function execution denied for workspace`, {
workspaceId,
userId: auth.userId,
})
return NextResponse.json(
{ success: false, error: 'Workspace access denied' },
{ status: 403 }
)
}

if (selectedSandboxId && !isRemoteSandboxEnabled) {
return NextResponse.json(
{ success: false, error: 'The Function code sandbox is not configured' },
Expand Down Expand Up @@ -2174,6 +2238,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
authUserId: auth.userId,
workflowId,
workspaceId,
workspaceAccess,
outputFiles,
exportedFiles,
exportedFileContent,
Expand Down Expand Up @@ -2357,6 +2422,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
authUserId: auth.userId,
workflowId,
workspaceId,
workspaceAccess,
outputFiles,
exportedFiles,
exportedFileContent,
Expand Down Expand Up @@ -2447,6 +2513,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
authUserId: auth.userId,
workflowId,
workspaceId,
workspaceAccess,
outputFiles,
exportedFiles,
exportedFileContent,
Expand Down
6 changes: 6 additions & 0 deletions apps/sim/lib/copilot/request/tools/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
type WorkspaceFileSecretProvenance,
type WorkspaceFileSecretProvenanceRepresentation,
} from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance'
import { checkWorkspaceAccess } from '@/lib/workspaces/permissions/utils'
import type { ResolvedSecretMatcher } from '@/executor/utils/resolved-secret-matcher'
import {
createResolvedSecretMatcher,
Expand Down Expand Up @@ -433,6 +434,10 @@ export async function maybeWriteOutputToFile(
}

const writtenFiles = []
// Resolved once so the writer's authorization check does not re-query per output file.
const workspaceAccess = preparedFiles.length
? await checkWorkspaceAccess(workspaceId, userId)
: undefined
for (const { outputFile, format, contentType, buffer, secretProvenance } of preparedFiles) {
if (context.abortSignal?.aborted) {
throw new Error('Request aborted before tool mutation could be applied')
Expand All @@ -441,6 +446,7 @@ export async function maybeWriteOutputToFile(
const written = await writeWorkspaceFileByPath({
workspaceId,
userId,
workspaceAccess,
target: {
path: outputFile.path,
mode: outputFile.mode ?? 'create',
Expand Down
Loading
Loading