Skip to content

feat: dynamic actors#4330

Draft
NathanFlurry wants to merge 2 commits intomainfrom
feat-dynamic-actors
Draft

feat: dynamic actors#4330
NathanFlurry wants to merge 2 commits intomainfrom
feat-dynamic-actors

Conversation

@NathanFlurry
Copy link
Member

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@NathanFlurry
Copy link
Member Author

NathanFlurry commented Feb 28, 2026

This change is part of the following stack:

Change managed by git-spice.

@railway-app
Copy link

railway-app bot commented Feb 28, 2026

🚅 Deployed to the rivet-pr-4330 environment in rivet-frontend

Service Status Web Updated (UTC)
website ❌ Build Failed (View Logs) Web Feb 28, 2026 at 5:28 pm
frontend-cloud ❌ Build Failed (View Logs) Web Feb 28, 2026 at 5:28 pm
frontend-inspector ❌ Build Failed (View Logs) Web Feb 28, 2026 at 5:28 pm
mcp-hub ✅ Success (View Logs) Web Feb 28, 2026 at 3:49 am
ladle ❌ Build Failed (View Logs) Web Feb 28, 2026 at 3:49 am

@pkg-pr-new
Copy link

pkg-pr-new bot commented Feb 28, 2026

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@4330

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@4330

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@4330

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@4330

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@4330

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@4330

@rivetkit/sqlite-vfs

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sqlite-vfs@4330

@rivetkit/traces

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/traces@4330

@rivetkit/workflow-engine

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/workflow-engine@4330

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@4330

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@4330

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@4330

commit: 72fe6b6

@claude
Copy link

claude bot commented Feb 28, 2026

PR Review: feat: dynamic actors

This is a substantial PR introducing dynamic actors — a sandboxing system that lets user-supplied TypeScript code run inside isolated-vm + sandboxed-node processes. The architecture is well-designed overall. Below are my findings.


Bugs

1. Typo in interface field name — likely a runtime crash
rivetkit-typescript/packages/rivetkit/dynamic-host-runtime/src/index.cts (interface definition) and host-runtime.ts reference __unsafeIsoalte (note the transposition of 'a' and 'l'). If the actual sandboxed-node API spells this __unsafeIsolate, this will crash at startup. Both references must match the real API surface.

2. Stale variable reference in commented-out example code
In examples/dynamic-actors/src/actors.ts (lines 430–440), the commented-out alternative dynamicActor loader uses sourceState.source but sourceState is never defined in that snippet — the snippet is supposed to await fetch(/* ... */) and use the result. This is confusing documentation. Either remove it or fix the variable reference.

3. Indentation inconsistency in global-state.ts
In the } else if (isStaticActorDefinition(...)) { branch of #startActorInner, the await staticActor.start(...) call is indented one extra level compared to its surrounding block. Minor but worth fixing for consistency.


Code Quality

4. Pervasive any in example actors
examples/dynamic-actors/src/actors.ts uses c: any in every action handler (lines 392, 396, 404). The whole point of the example is to show the API, so it should use proper typed context parameters. Using any here sets a bad precedent for users copying this pattern.

5. Left-over commented-out code in example
The large commented block at the bottom of examples/dynamic-actors/src/actors.ts (lines 426–445) should be removed before merging. It's dead code that adds noise without teaching anything (the fetch URL is a placeholder).

6. Missing skipVercel: true in example package.json
Per CLAUDE.md, new examples must either generate a Vercel equivalent (by running ./scripts/vercel-examples/generate-vercel-examples.ts) or add "skipVercel": true to the template object in package.json. The examples/dynamic-actors/package.json has neither. This example likely can't run on Vercel (requires sandboxed-node), so skipVercel: true should be added.

7. createWithInput is unused in the source actor
examples/dynamic-actors/src/server.ts passes createWithInput: { source: DEFAULT_DYNAMIC_ACTOR_SOURCE } to getOrCreate, but sourceCode never reads input — its initial state is hardcoded in the state definition in actors.ts. The createWithInput call will be silently ignored.


Security / Sandboxing

8. File system fallback allows read access to host node_modules
In buildLockedDownPermissions, the fs policy allows read-only access to fallbackNodeModules (the parent directory's node_modules) and rootNodeModules (/node_modules). A malicious dynamic actor can enumerate and read all packages installed on the host — including any that contain secrets in their install-time scripts, config files, or cached credentials. Consider making this an explicit allowlist of package names rather than an open directory read.

9. isPathWithin does not resolve symlinks
The path containment check uses path.resolve, which follows symlinks in the candidate path segments but not in arbitrary intermediate directory symlinks. On some configurations, a crafted relative path or a package with a symlinked entry point could escape the sandbox directory boundary. Consider using realpath (or fs.realpathSync) for both candidate and parent before comparison.

10. Environment variable claim needs verification
The comment at line ~1765 in host-runtime.ts states "Dynamic actors only receive explicitly injected env vars". The explicitly injected set is { HOME, XDG_DATA_HOME, XDG_CACHE_HOME, TMPDIR, RIVET_EXPOSE_ERRORS }. The env: () => ({ allow: true }) permission means the sandbox CAN access environment variables — but only ones passed via processConfig.env. This claim is correct if sandboxed-node truly shadows process.env rather than inheriting it. Worth adding a test or explicit assertion to confirm this property, since it's a critical security invariant.

11. No default resource limits
DynamicNodeProcessConfig makes memoryLimit and cpuTimeLimitMs optional, and the runtime passes them through as undefined if the loader doesn't set them. If sandboxed-node has no built-in defaults, a loader that omits these values could run unbounded. Consider enforcing minimum safe defaults (e.g., 256 MB / 30s) when the loader does not provide them.


Architecture

12. Repeated isDynamicActor lookup on every request
FileSystemManagerDriver calls isDynamicActor (which does a storage read) at the top of every HTTP request and WebSocket open. Since the actor type is fixed at creation time, this could be cached in-memory on first resolution to avoid repeated I/O.

13. AnyActorInstance vs AnyStaticActorInstance rename is good — consider exporting a union
The refactor correctly narrows inspector/workflow types to AnyStaticActorInstance. It might be worth exporting a AnyActorInstance = AnyStaticActorInstance | DynamicActorInstance union from instance/mod.ts so call sites that genuinely accept both don't have to import from two places.


Minor

  • examples/dynamic-actors/src/server.ts: the /api/rivet/* passthrough route comment-less — a short comment explaining this proxies RivetKit's internal endpoints would help readers unfamiliar with the pattern.
  • dynamic-host-runtime/src/index.cts top-level /* eslint-disable @typescript-eslint/no-explicit-any */ is broad; narrowing it to the specific lines or adding inline overrides where needed would be cleaner.

Summary

The dynamic actor sandboxing model is well-structured: explicit host bridge injection, data serialized across isolation boundaries, and network/child-process access denied. The main concerns before merging are the likely typo in __unsafeIsoalte, the missing Vercel skip flag, and the security-relevant file system fallback scope. The example code quality (dead code, any types) should also be cleaned up before it ships as documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant