diff --git a/.claude/rules/global.md b/.claude/rules/global.md index b80c3695ce3..4c98edc16cb 100644 --- a/.claude/rules/global.md +++ b/.claude/rules/global.md @@ -30,5 +30,25 @@ const shortId = generateShortId() const tiny = generateShortId(8) ``` +## Common Utilities +Use shared helpers from `@/lib/core/utils/helpers` instead of writing inline implementations: + +- `sleep(ms)` — async delay. Never write `new Promise(resolve => setTimeout(resolve, ms))` +- `toError(value)` — normalize unknown caught values to `Error`. Never write `e instanceof Error ? e : new Error(String(e))` +- `toError(value).message` — get error message safely. Never write `e instanceof Error ? e.message : String(e)` + +```typescript +// ✗ Bad +await new Promise(resolve => setTimeout(resolve, 1000)) +const msg = error instanceof Error ? error.message : String(error) +const err = error instanceof Error ? error : new Error(String(error)) + +// ✓ Good +import { sleep, toError } from '@/lib/core/utils/helpers' +await sleep(1000) +const msg = toError(error).message +const err = toError(error) +``` + ## Package Manager Use `bun` and `bunx`, not `npm` and `npx`. diff --git a/.cursor/rules/global.mdc b/.cursor/rules/global.mdc index af32f057955..991244503f3 100644 --- a/.cursor/rules/global.mdc +++ b/.cursor/rules/global.mdc @@ -37,5 +37,25 @@ const shortId = generateShortId() const tiny = generateShortId(8) ``` +## Common Utilities +Use shared helpers from `@/lib/core/utils/helpers` instead of writing inline implementations: + +- `sleep(ms)` — async delay. Never write `new Promise(resolve => setTimeout(resolve, ms))` +- `toError(value)` — normalize unknown caught values to `Error`. Never write `e instanceof Error ? e : new Error(String(e))` +- `toError(value).message` — get error message safely. Never write `e instanceof Error ? e.message : String(e)` + +```typescript +// ✗ Bad +await new Promise(resolve => setTimeout(resolve, 1000)) +const msg = error instanceof Error ? error.message : String(error) +const err = error instanceof Error ? error : new Error(String(error)) + +// ✓ Good +import { sleep, toError } from '@/lib/core/utils/helpers' +await sleep(1000) +const msg = toError(error).message +const err = toError(error) +``` + ## Package Manager Use `bun` and `bunx`, not `npm` and `npx`. diff --git a/.cursor/rules/sim-sandbox.mdc b/.cursor/rules/sim-sandbox.mdc new file mode 100644 index 00000000000..74ba0ffc85f --- /dev/null +++ b/.cursor/rules/sim-sandbox.mdc @@ -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. diff --git a/CLAUDE.md b/CLAUDE.md index bc54c6f912c..965ae7fd7b3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,6 +8,7 @@ You are a professional software engineer. All code must follow best practices: a - **Comments**: Use TSDoc for documentation. No `====` separators. No non-TSDoc comments - **Styling**: Never update global styles. Keep all styling local to components - **ID Generation**: Never use `crypto.randomUUID()`, `nanoid`, or `uuid` package. Use `generateId()` (UUID v4) or `generateShortId()` (compact) from `@/lib/core/utils/uuid` +- **Common Utilities**: Use shared helpers from `@/lib/core/utils/helpers` instead of inline implementations. `sleep(ms)` for delays, `toError(e)` to normalize caught values. - **Package Manager**: Use `bun` and `bunx`, not `npm` and `npx` ## Architecture diff --git a/apps/docs/components/ui/action-media.tsx b/apps/docs/components/ui/action-media.tsx index 629716b6640..77b885f1527 100644 --- a/apps/docs/components/ui/action-media.tsx +++ b/apps/docs/components/ui/action-media.tsx @@ -1,6 +1,6 @@ 'use client' -import { useState } from 'react' +import { useRef, useState } from 'react' import { cn, getAssetUrl } from '@/lib/utils' import { Lightbox } from './lightbox' @@ -50,11 +50,14 @@ export function ActionImage({ src, alt, enableLightbox = true }: ActionImageProp } export function ActionVideo({ src, alt, enableLightbox = true }: ActionVideoProps) { + const videoRef = useRef(null) + const startTimeRef = useRef(0) const [isLightboxOpen, setIsLightboxOpen] = useState(false) const resolvedSrc = getAssetUrl(src) const handleClick = () => { if (enableLightbox) { + startTimeRef.current = videoRef.current?.currentTime ?? 0 setIsLightboxOpen(true) } } @@ -62,6 +65,7 @@ export function ActionVideo({ src, alt, enableLightbox = true }: ActionVideoProp return ( <>