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
85 changes: 85 additions & 0 deletions .cursor/rules/sim-sandbox.mdc
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
---
description: Isolated-vm sandbox worker security policy. Hard rules for anything that lives in the worker child process that runs user code.
globs: ["apps/sim/lib/execution/isolated-vm-worker.cjs", "apps/sim/lib/execution/isolated-vm.ts", "apps/sim/lib/execution/sandbox/**", "apps/sim/sandbox-tasks/**"]
---

# Sim Sandbox — Worker Security Policy

The isolated-vm worker child process at
`apps/sim/lib/execution/isolated-vm-worker.cjs` runs untrusted user code inside
V8 isolates. The process itself is a trust boundary. Everything in this rule is
about what must **never** live in that process.

## Hard rules

1. **No app credentials in the worker process**. The worker must not hold, load,
or receive via IPC: database URLs, Redis URLs, AWS keys, Stripe keys,
session-signing keys, encryption keys, OAuth client secrets, internal API
secrets, or any LLM / email / search provider API keys. If you catch yourself
`require`'ing `@/lib/auth`, `@sim/db`, `@/lib/uploads/core/storage-service`,
or anything that imports `env` directly inside the worker, stop and use a
host-side broker instead.

2. **Host-side brokers own all credentialed work**. The worker can only access
resources through `ivm.Reference` / `ivm.Callback` bridges back to the host
process. Today the only broker is `workspaceFileBroker`
(`apps/sim/lib/execution/sandbox/brokers/workspace-file.ts`); adding a new
one requires co-reviewing this file.

3. **Host-side brokers must scope every resource access to a single tenant**.
The `SandboxBrokerContext` always carries `workspaceId`. Any new broker that
accesses storage, DB, or an external API must use `ctx.workspaceId` to scope
the lookup — never accept a raw path, key, or URL from isolate code without
validation.

4. **Nothing that runs in the isolate is trusted, even if we wrote it**. The
task `bootstrap` and `finalize` strings in `apps/sim/sandbox-tasks/` execute
inside the isolate. They must treat `globalThis` as adversarial — no pulling
values from it that might have been mutated by user code. The hardening
script in `executeTask` undefines dangerous globals before user code runs.

## Why

A V8 JIT bug (Chrome ships these roughly monthly) gives an attacker a native
code primitive inside the process that owns whatever that process can reach.
If the worker only holds `isolated-vm` + a single narrow workspace-file broker,
a V8 escape leaks one tenant's files. If the worker holds a Stripe key or a DB
connection, a V8 escape leaks the service.

The original `doc-worker.cjs` vulnerability (CVE-class, 225 production secrets
leaked via `/proc/1/environ`) was the forcing function for this architecture.
Keep the blast radius small.

## Checklist for changes to `isolated-vm-worker.cjs`

Before landing any change that adds a new `require(...)` or `process.send(...)`
payload or `ivm.Reference` wrapper in the worker:

- [ ] Does it load a credential, key, connection string, or secret? If yes,
move it host-side and expose as a broker.
- [ ] Does it import from `@/lib/auth`, `@sim/db`, `@/lib/uploads/core/*`,
`@/lib/core/config/env`, or any module that reads `process.env` of the
main app? If yes, same — move host-side.
- [ ] Does it expose a resource that's workspace-scoped without taking a
`workspaceId`? If yes, re-scope.
- [ ] Did you update the broker limits (`IVM_MAX_BROKER_ARGS_JSON_CHARS`,
`IVM_MAX_BROKER_RESULT_JSON_CHARS`, `IVM_MAX_BROKERS_PER_EXECUTION`) if
the new broker can emit large payloads or fire frequently?

## What the worker *may* hold

- `isolated-vm` module
- Node built-ins: `node:fs` (only for reading the checked-in bundle `.cjs`
files) and `node:path`
- The three prebuilt library bundles under
`apps/sim/lib/execution/sandbox/bundles/*.cjs`
- IPC message handlers for `execute`, `cancel`, `fetchResponse`,
`brokerResponse`

The worker deliberately has **no host-side logger**. All errors and
diagnostics flow through IPC back to the host, which has `@sim/logger`. Do
not add `createLogger` or console-based logging to the worker — it would
require pulling the main app's config / env, which is exactly what this
rule is preventing.

Anything else is suspect.
10 changes: 6 additions & 4 deletions apps/sim/app/api/files/serve/[...path]/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,12 @@ vi.mock('@/lib/uploads/utils/file-utils', () => ({

vi.mock('@/lib/uploads/setup.server', () => ({}))

vi.mock('@/lib/execution/doc-vm', () => ({
generatePdfFromCode: vi.fn().mockResolvedValue(Buffer.from('%PDF-compiled')),
generateDocxFromCode: vi.fn().mockResolvedValue(Buffer.from('PK\x03\x04compiled')),
generatePptxFromCode: vi.fn().mockResolvedValue(Buffer.from('PK\x03\x04compiled')),
vi.mock('@/lib/execution/sandbox/run-task', () => ({
runSandboxTask: vi
.fn()
.mockImplementation(async (taskId: string) =>
taskId === 'pdf-generate' ? Buffer.from('%PDF-compiled') : Buffer.from('PK\x03\x04compiled')
),
}))

vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({
Expand Down
47 changes: 29 additions & 18 deletions apps/sim/app/api/files/serve/[...path]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ import { createLogger } from '@sim/logger'
import type { NextRequest } from 'next/server'
import { NextResponse } from 'next/server'
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
import {
generateDocxFromCode,
generatePdfFromCode,
generatePptxFromCode,
} from '@/lib/execution/doc-vm'
import { runSandboxTask } from '@/lib/execution/sandbox/run-task'
import { CopilotFiles, isUsingCloudStorage } from '@/lib/uploads'
import type { StorageContext } from '@/lib/uploads/config'
import { parseWorkspaceFileKey } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
Expand All @@ -22,6 +18,7 @@ import {
findLocalFile,
getContentType,
} from '@/app/api/files/utils'
import type { SandboxTaskId } from '@/sandbox-tasks/registry'

const logger = createLogger('FilesServeAPI')

Expand All @@ -30,24 +27,24 @@ const PDF_MAGIC = Buffer.from([0x25, 0x50, 0x44, 0x46, 0x2d]) // %PDF-

interface CompilableFormat {
magic: Buffer
compile: (code: string, workspaceId: string) => Promise<Buffer>
taskId: SandboxTaskId
contentType: string
}

const COMPILABLE_FORMATS: Record<string, CompilableFormat> = {
'.pptx': {
magic: ZIP_MAGIC,
compile: generatePptxFromCode,
taskId: 'pptx-generate',
contentType: 'application/vnd.openxmlformats-officedocument.presentationml.presentation',
},
'.docx': {
magic: ZIP_MAGIC,
compile: generateDocxFromCode,
taskId: 'docx-generate',
contentType: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
},
'.pdf': {
magic: PDF_MAGIC,
compile: generatePdfFromCode,
taskId: 'pdf-generate',
contentType: 'application/pdf',
},
}
Expand All @@ -65,8 +62,10 @@ function compiledCacheSet(key: string, buffer: Buffer): void {
async function compileDocumentIfNeeded(
buffer: Buffer,
filename: string,
workspaceId?: string,
raw?: boolean
workspaceId: string | undefined,
raw: boolean,
ownerKey: string | undefined,
signal: AbortSignal | undefined
): Promise<{ buffer: Buffer; contentType: string }> {
if (raw) return { buffer, contentType: getContentType(filename) }

Expand All @@ -90,7 +89,11 @@ async function compileDocumentIfNeeded(
return { buffer: cached, contentType: format.contentType }
}

const compiled = await format.compile(code, workspaceId || '')
const compiled = await runSandboxTask(
format.taskId,
{ code, workspaceId: workspaceId || '' },
{ ownerKey, signal }
)
compiledCacheSet(cacheKey, compiled)
return { buffer: compiled, contentType: format.contentType }
}
Expand Down Expand Up @@ -153,10 +156,10 @@ export async function GET(
const userId = authResult.userId

if (isUsingCloudStorage()) {
return await handleCloudProxy(cloudKey, userId, raw)
return await handleCloudProxy(cloudKey, userId, raw, request.signal)
}

return await handleLocalFile(cloudKey, userId, raw)
return await handleLocalFile(cloudKey, userId, raw, request.signal)
} catch (error) {
logger.error('Error serving file:', error)

Expand All @@ -171,8 +174,10 @@ export async function GET(
async function handleLocalFile(
filename: string,
userId: string,
raw: boolean
raw: boolean,
signal: AbortSignal | undefined
): Promise<NextResponse> {
const ownerKey = `user:${userId}`
try {
const contextParam: StorageContext | undefined = inferContextFromKey(filename) as
| StorageContext
Expand Down Expand Up @@ -205,7 +210,9 @@ async function handleLocalFile(
rawBuffer,
displayName,
workspaceId,
raw
raw,
ownerKey,
signal
)

logger.info('Local file served', { userId, filename, size: fileBuffer.length })
Expand All @@ -225,8 +232,10 @@ async function handleLocalFile(
async function handleCloudProxy(
cloudKey: string,
userId: string,
raw = false
raw = false,
signal: AbortSignal | undefined = undefined
): Promise<NextResponse> {
const ownerKey = `user:${userId}`
try {
const context = inferContextFromKey(cloudKey)
logger.info(`Inferred context: ${context} from key pattern: ${cloudKey}`)
Expand Down Expand Up @@ -262,7 +271,9 @@ async function handleCloudProxy(
rawBuffer,
displayName,
workspaceId,
raw
raw,
ownerKey,
signal
)

logger.info('Cloud file served', {
Expand Down
8 changes: 6 additions & 2 deletions apps/sim/app/api/workspaces/[id]/pptx/preview/route.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { createLogger } from '@sim/logger'
import { type NextRequest, NextResponse } from 'next/server'
import { getSession } from '@/lib/auth'
import { generatePptxFromCode } from '@/lib/execution/doc-vm'
import { runSandboxTask } from '@/lib/execution/sandbox/run-task'
import { verifyWorkspaceMembership } from '@/app/api/workflows/utils'

export const dynamic = 'force-dynamic'
Expand Down Expand Up @@ -44,7 +44,11 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id:
return NextResponse.json({ error: 'code exceeds maximum size' }, { status: 413 })
}

const buffer = await generatePptxFromCode(code, workspaceId, req.signal)
const buffer = await runSandboxTask(
'pptx-generate',
{ code, workspaceId },
{ ownerKey: `user:${session.user.id}`, signal: req.signal }
)

return new NextResponse(new Uint8Array(buffer), {
status: 200,
Expand Down
48 changes: 7 additions & 41 deletions apps/sim/lib/copilot/tools/server/files/edit-content.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,10 @@ import {
type ServerToolContext,
} from '@/lib/copilot/tools/server/base-tool'
import { toError } from '@/lib/core/utils/helpers'
import {
generateDocxFromCode,
generatePdfFromCode,
generatePptxFromCode,
} from '@/lib/execution/doc-vm'
import { runSandboxTask } from '@/lib/execution/sandbox/run-task'
import { updateWorkspaceFileContent } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
import { consumeLatestFileIntent } from './file-intent-store'
import { inferContentType } from './workspace-file'
import { getDocumentFormatInfo, inferContentType } from './workspace-file'

const logger = createLogger('EditContentServerTool')

Expand All @@ -26,40 +22,6 @@ type EditContentResult = {
data?: Record<string, unknown>
}

function getDocumentFormatInfo(fileName: string): {
isDoc: boolean
formatName?: string
sourceMime?: string
generator?: (code: string, workspaceId: string, signal?: AbortSignal) => Promise<Buffer>
} {
const lowerName = fileName.toLowerCase()
if (lowerName.endsWith('.pptx')) {
return {
isDoc: true,
formatName: 'PPTX',
sourceMime: 'text/x-pptxgenjs',
generator: generatePptxFromCode,
}
}
if (lowerName.endsWith('.docx')) {
return {
isDoc: true,
formatName: 'DOCX',
sourceMime: 'text/x-docxjs',
generator: generateDocxFromCode,
}
}
if (lowerName.endsWith('.pdf')) {
return {
isDoc: true,
formatName: 'PDF',
sourceMime: 'text/x-pdflibjs',
generator: generatePdfFromCode,
Comment thread
icecrasher321 marked this conversation as resolved.
}
}
return { isDoc: false }
}

export const editContentServerTool: BaseServerTool<EditContentArgs, EditContentResult> = {
name: 'edit_content',
async execute(params: EditContentArgs, context?: ServerToolContext): Promise<EditContentResult> {
Expand Down Expand Up @@ -241,7 +203,11 @@ export const editContentServerTool: BaseServerTool<EditContentArgs, EditContentR

if (docInfo.isDoc) {
try {
await docInfo.generator!(finalContent, workspaceId)
await runSandboxTask(
docInfo.taskId!,
{ code: finalContent, workspaceId },
{ ownerKey: `user:${context.userId}`, signal: context.abortSignal }
)
} catch (err) {
Comment thread
icecrasher321 marked this conversation as resolved.
const msg = toError(err).message
return {
Expand Down
Loading
Loading