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
9 changes: 8 additions & 1 deletion .claude/rules/sim-sandbox.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ about what must **never** live in that process.
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.
host-side broker instead. This includes the OS environment: the worker is
spawned with the explicit allowlisted env from `buildWorkerEnv()` in
`isolated-vm.ts` — never spawn it without an `env` option (Node would copy
the app's full `process.env`, secrets included, into the worker).

2. **Host-side brokers own all credentialed work**. The worker can only access
resources through `ivm.Reference` / `ivm.Callback` bridges back to the host
Expand Down Expand Up @@ -69,6 +72,10 @@ payload or `ivm.Reference` wrapper in the worker:
- [ ] 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?
- [ ] Does the worker read a new env var? Add it to the `buildWorkerEnv()`
allowlist in `isolated-vm.ts` **and** to the allowlist regression test in
`isolated-vm.test.ts` — the worker does not inherit the app environment,
so an un-allowlisted var is simply absent in the child.

## What the worker *may* hold

Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/test-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ jobs:
- name: Setup Node
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6
with:
node-version: 22
node-version: 24

# Cache keys are scoped by event name, and fork PRs get their own
# namespace on top: untrusted fork runs must never share a cache with
Expand Down Expand Up @@ -250,7 +250,7 @@ jobs:
- name: Setup Node
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6
with:
node-version: 22
node-version: 24

- name: Mount Bun cache
uses: ./.github/actions/cache-mount
Expand Down
62 changes: 62 additions & 0 deletions apps/sim/lib/execution/isolated-vm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,68 @@ describe('isolated-vm scheduler', () => {
expect(spawnMock).toHaveBeenCalledTimes(2)
})

it('spawns workers with an allowlisted env, never the parent process.env', async () => {
const ALLOWED_WORKER_ENV_KEYS = new Set([
'PATH',
'NODE_ENV',
'IVM_MAX_STDOUT_CHARS',
'IVM_MAX_FETCH_OPTIONS_JSON_CHARS',
'TZ',
'LANG',
'LC_ALL',
'SYSTEMROOT',
'WINDIR',
'COMSPEC',
'PATHEXT',
'TEMP',
'TMP',
])
vi.stubEnv('SIM_SANDBOX_SECRET_CANARY', 'must-not-reach-worker')
try {
const { executeInIsolatedVM, spawnMock } = await loadExecutionModule({
spawns: [() => createReadyProc('ok')],
})

await executeInIsolatedVM({
code: 'return "ok"',
params: {},
envVars: {},
contextVariables: {},
timeoutMs: 100,
requestId: 'req-env',
})

expect(spawnMock).toHaveBeenCalledTimes(1)
const spawnOptions = spawnMock.mock.calls[0]?.[2] as
| { env?: Record<string, string> }
| undefined
expect(spawnOptions?.env).toBeDefined()
const workerEnv = spawnOptions?.env ?? {}

expect(workerEnv.SIM_SANDBOX_SECRET_CANARY).toBeUndefined()
for (const secretKey of [
'DATABASE_URL',
'REDIS_URL',
'ENCRYPTION_KEY',
'BETTER_AUTH_SECRET',
'STRIPE_SECRET_KEY',
'OPENAI_API_KEY',
'AWS_SECRET_ACCESS_KEY',
]) {
expect(workerEnv[secretKey]).toBeUndefined()
}
for (const key of Object.keys(workerEnv)) {
expect(
ALLOWED_WORKER_ENV_KEYS.has(key),
`unexpected env var forwarded to sandbox worker: ${key}`
).toBe(true)
}
expect(workerEnv.PATH).toBe(process.env.PATH)
} finally {
vi.unstubAllEnvs()
}
})

it('rejects new requests when the queue is full', async () => {
const holder = createControllableReadyProc()
const { executeInIsolatedVM } = await loadExecutionModule({
Expand Down
33 changes: 33 additions & 0 deletions apps/sim/lib/execution/isolated-vm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import path from 'node:path'
import { fileURLToPath } from 'node:url'
import { createLogger } from '@sim/logger'
import { getErrorMessage, toError } from '@sim/utils/errors'
import { filterUndefined } from '@sim/utils/object'
import { randomFloat } from '@sim/utils/random'
import { env } from '@/lib/core/config/env'
import { getRedisClient } from '@/lib/core/config/redis'
Expand Down Expand Up @@ -890,6 +891,37 @@ function resetWorkerIdleTimeout(workerId: number) {
}
}

/**
* Environment for the sandbox worker process. The worker runs untrusted user
* code, so it must never inherit the app's `process.env` (DB URLs, encryption
* keys, provider API keys — see `.claude/rules/sim-sandbox.md`): a V8 isolate
* escape would read every inherited secret from the worker's environment.
* Only an explicit allowlist is forwarded — `PATH` so `spawn('node', ...)` can
* resolve the binary, `NODE_ENV`, the two `IVM_*` limits the worker reads,
* timezone/locale vars so `Date`/`Intl` behavior inside isolates matches the
* host, and the Windows system vars Node needs to boot (undefined elsewhere
* and stripped).
* Any new env var the worker reads must be added here and to the allowlist
* regression test in `isolated-vm.test.ts`.
*/
function buildWorkerEnv(): NodeJS.ProcessEnv {
const allowed: Record<string, string | undefined> = {
PATH: process.env.PATH,
IVM_MAX_STDOUT_CHARS: env.IVM_MAX_STDOUT_CHARS,
IVM_MAX_FETCH_OPTIONS_JSON_CHARS: env.IVM_MAX_FETCH_OPTIONS_JSON_CHARS,
TZ: process.env.TZ,
LANG: process.env.LANG,
LC_ALL: process.env.LC_ALL,
SYSTEMROOT: process.env.SYSTEMROOT,
WINDIR: process.env.WINDIR,
COMSPEC: process.env.COMSPEC,
PATHEXT: process.env.PATHEXT,
TEMP: process.env.TEMP,
TMP: process.env.TMP,
}
return { ...filterUndefined(allowed), NODE_ENV: process.env.NODE_ENV }
}

function spawnWorker(): Promise<WorkerInfo> {
const workerId = nextWorkerId++
spawnInProgress++
Expand Down Expand Up @@ -955,6 +987,7 @@ function spawnWorker(): Promise<WorkerInfo> {
const proc = spawn('node', ['--no-node-snapshot', workerPath], {
stdio: ['ignore', 'pipe', 'pipe', 'ipc'],
serialization: 'json',
env: buildWorkerEnv(),
})
childProcess = proc

Expand Down
2 changes: 1 addition & 1 deletion apps/sim/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@
"input-otp": "^1.4.2",
"ioredis": "^5.6.0",
"ipaddr.js": "2.3.0",
"isolated-vm": "6.0.2",
"isolated-vm": "6.1.2",
"jose": "6.0.11",
"js-tiktoken": "1.0.21",
"js-yaml": "4.3.0",
Expand Down
Loading
Loading