Too many files changed for review. (134 files found, 100 file limit)

@aisle-research-bot review

🔒 Aisle Security Analysis

We found 14 potential security issue(s) in this PR:

1. 🔴 Unauthenticated, origin-unrestricted VNC WebSocket proxy exposed on all interfaces

Description

The host-side VNC WebSocket proxy allows any website/client to open a WebSocket connection and bridge directly into a sandbox VM's VNC server.

Key issues:

• Cross-Site WebSocket Hijacking (CSWSH): CheckOrigin unconditionally returns true, so the browser's Origin is not validated.
• Missing authentication/authorization: access control is only a sandbox_id query parameter. Any party that can reach the HTTP port can attempt to connect to arbitrary sandboxes.
• Network exposure by default: the proxy is started with http.ListenAndServe(":<port>", ...), binding to all interfaces. Combined with the above, this can become unauthenticated remote desktop access to VMs.

Vulnerable code:

upgrader: websocket.Upgrader{
  Subprotocols: []string{"binary"},
  CheckOrigin:  func(r *http.Request) bool { return true },
}

sandboxID := r.URL.Query().Get("sandbox_id")
...
wsConn, err := p.upgrader.Upgrade(w, r, nil)

Related exposure point:

• cmd/vm-runner/main.go starts the HTTP server on ":<port>" and registers /vnc without any auth middleware.

Recommendation

Add both (1) strong authorization and (2) strict origin protection; also reduce network exposure.

1. Bind to localhost by default (or make explicit):

addr := fmt.Sprintf("127.0.0.1:%d", cfg.VNCProxyPort)
httpSrv := &http.Server{Addr: addr, Handler: httpMux}
log.Fatal(httpSrv.ListenAndServe())

2. Require an unguessable per-sandbox/session token (not just sandbox_id) and validate it server-side.

3. Implement strict CheckOrigin (allow only your UI origin(s); reject missing/invalid origins):

upgrader := websocket.Upgrader{
  Subprotocols: []string{"binary"},
  CheckOrigin: func(r *http.Request) bool {
    origin := r.Header.Get("Origin")
    return origin == "https://your-ui.example" || origin == "http://127.0.0.1:yourport"
  },
}

4. Consider serving the proxy behind TLS (wss://) and an authenticated reverse proxy if it must be remotely reachable.

2. 🟠 DoS risk in VNC WebSocket proxy due to unlimited message sizes and missing read/write timeouts

Description

The VNC WebSocket proxy forwards WebSocket frames to a VNC connection without applying defensive limits.

Impacts:

• Memory exhaustion: wsConn.ReadMessage() reads an entire message into memory with no SetReadLimit, allowing a client to send very large frames.
• Goroutine/socket leak / slowloris: no read deadlines or write deadlines are set on the WebSocket connection. A client can stop reading (or trickle data) causing WriteMessage / ReadMessage to block indefinitely, tying up goroutines and file descriptors.

Vulnerable code:

for {
    _, msg, err := wsConn.ReadMessage()
    ...
    if _, err := vncConn.Write(msg); err != nil { ... }
}
...
if wErr := wsConn.WriteMessage(websocket.BinaryMessage, buf[:n]); wErr != nil { ... }

Recommendation

Add defensive WebSocket limits and deadlines.

Example hardening:

const (
  maxMsgSize   = 1 << 20 // 1 MiB, tune for noVNC
  pongWait     = 30 * time.Second
  writeWait    = 10 * time.Second
)

wsConn.SetReadLimit(maxMsgSize)
_ = wsConn.SetReadDeadline(time.Now().Add(pongWait))
wsConn.SetPongHandler(func(string) error {
  return wsConn.SetReadDeadline(time.Now().Add(pongWait))
})

​// Before each write
_ = wsConn.SetWriteDeadline(time.Now().Add(writeWait))
if err := wsConn.WriteMessage(websocket.BinaryMessage, payload); err != nil {
  return
}

Also consider:

• Closing both sides on first error and waiting for both goroutines to exit (e.g., via context.Context/sync.WaitGroup).
• Rate limiting connections per IP if the proxy is reachable by untrusted clients.

3. 🟠 SSRF protection bypass via DNS hostname resolution in URLPolicy.Validate

Description

URLPolicy.Validate is intended to prevent SSRF by blocking navigation to localhost, cloud metadata, and private ranges. However, it only blocks IP literals (including decimal/hex encodings) and does not resolve hostnames to their IP addresses.

As a result:

• An attacker can supply a hostname (e.g. http://evil.example/) that resolves to a blocked address such as 169.254.169.254 (cloud metadata), 127.0.0.1, or RFC1918 space.
• This also enables DNS rebinding style attacks where the hostname initially resolves to a public IP (passing any naive checks) and later resolves to an internal IP when Chromium actually connects.

Vulnerable code:

​// ...
​// Try parsing as IP literals (dotted, decimal, hex)
​// ...
return nil // all non-IP hostnames are allowed without DNS resolution

Recommendation

Resolve hostnames and block if any resolved address falls into a blocked range. Additionally, mitigate DNS rebinding by re-resolving close to connection time and/or pinning the resolved IP(s) for the navigation.

Example (simplified):

func (p *URLPolicy) Validate(ctx context.Context, rawURL string) error {
    ​// ... existing parsing and scheme checks ...

    host := parsed.Hostname()

    ​// Block IP literals as today
    if ip := net.ParseIP(host); ip != nil {
        if isBlockedIP(ip) { return fmt.Errorf("blocked IP: %s", host) }
        return nil
    }

    ​// NEW: resolve DNS
    ips, err := net.DefaultResolver.LookupIP(ctx, "ip", host)
    if err != nil {
        return fmt.Errorf("dns lookup failed: %w", err)
    }
    for _, ip := range ips {
        if isBlockedIP(ip) {
            return fmt.Errorf("blocked hostname %s resolves to blocked IP %s", host, ip.String())
        }
    }
    return nil
}

For stronger SSRF defenses, consider enforcing egress restrictions at the network layer (guest firewall / host netns) so DNS tricks and redirects cannot reach link-local/private ranges even if application logic is bypassed.

4. 🟠 SSRF via redirects not blocked in envd BrowserService.Navigate (Go/chromedp)

Description

The envd BrowserService.Navigate RPC performs URL validation only on the initial user-supplied URL, then delegates navigation to Chromium (via chromedp.Navigate). Chromium will follow HTTP redirects, but the code does not re-validate the redirect target before the request is made.

This enables SSRF bypasses such as:

• Attacker provides a public URL https://attacker.example/redirect that responds 302 Location: http://169.254.169.254/latest/meta-data/
• URLPolicy.Validate() is applied to the original public URL and passes
• Chromium follows the redirect and performs the request to a blocked internal/metadata IP

Vulnerable code:

if err := s.urlPolicy.Validate(req.GetUrl()); err != nil {
    return nil, status.Errorf(codes.InvalidArgument, "blocked URL: %v", err)
}
...
if err := chromedp.Run(runCtx, chromedp.Navigate(req.GetUrl())); err != nil {
    return nil, status.Errorf(codes.Internal, "navigate: %v", err)
}

Recommendation

Enforce SSRF protection at the network request level (redirects happen inside the browser), not just on the initial URL string.

Options (can be combined):

1. Intercept/deny requests in Chromium via CDP and block by destination IP:

   • Enable Fetch or Network domain events
   • For each outgoing request, resolve the request URL hostname to IP(s) and block private/link-local/metadata ranges
   • Abort blocked requests using Fetch.failRequest

2. Add egress filtering in the VM (iptables/nftables) to deny traffic to private/link-local/metadata IP ranges (IPv4 and IPv6). This prevents redirects/DNS rebinding/alternate URL forms from reaching restricted addresses even if the browser tries.

At minimum, log and reject sessions that end up at a blocked URL after navigation (defense-in-depth), but note this alone does not prevent the request from being made.

5. 🟠 Unrestricted filesystem access in envd FileService (arbitrary path read/write/delete via gRPC)

Description

The new guest-side envd gRPC FileService performs filesystem operations directly on the caller-supplied path with no workspace-root enforcement or safe path resolution.

This enables a client that can reach envd (via the host vm-runner proxy) to:

• Read arbitrary files by passing absolute paths (e.g. /etc/shadow, /proc/self/environ)
• Write/overwrite arbitrary files (including creating missing parent dirs) by passing attacker-chosen paths
• Recursively delete arbitrary directories by calling Remove(recursive=true) on attacker-chosen paths
• Escape any intended “workspace-only” confinement (e.g. /workspace) via absolute paths, .. segments, or symlink components in the provided path

Vulnerable code (examples):

​// Read: arbitrary path
f, err := os.Open(req.GetPath())

​// Write: arbitrary path + auto-create parent dirs
filePath = req.GetPath()
dir := filepath.Dir(filePath)
os.MkdirAll(dir, 0755)
f, err = os.OpenFile(filePath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, mode)

​// Delete: arbitrary path (including recursive)
os.RemoveAll(req.GetPath())

Note: openclaw-vm-runner/internal/server/file_service.go forwards user-controlled paths from the host-side gRPC API directly to this envd service, so the lack of validation is reachable from the host-side sandbox FileService as well.

Recommendation

Restrict all filesystem RPCs to a configured base directory (e.g. /workspace) and perform symlink-safe path joining.

Recommended approach:

• Require relative paths from the client (reject absolute paths)
• Clean paths and reject any that escape the base (e.g. ..)
• Use a symlink-safe join (e.g. github.com/cyphar/filepath-securejoin) to prevent symlink component escapes
• Apply the same resolver for Stat/ReadFile/WriteFile/ListDir/MakeDir/Remove

Example:

import (
  "os"
  "path/filepath"
  securejoin "github.com/cyphar/filepath-securejoin"
)

const workspaceRoot = "/workspace"

func resolvePath(userPath string) (string, error) {
  if userPath == "" {
    return "", status.Error(codes.InvalidArgument, "path required")
  }
  if filepath.IsAbs(userPath) {
    return "", status.Error(codes.PermissionDenied, "absolute paths are not allowed")
  }
  ​// securejoin defends against symlink components escaping the root
  full, err := securejoin.SecureJoin(workspaceRoot, userPath)
  if err != nil {
    return "", status.Error(codes.InvalidArgument, "invalid path")
  }
  return full, nil
}

​// usage in ReadFile:
fullPath, err := resolvePath(req.GetPath())
if err != nil { return err }
f, err := os.Open(fullPath)

Additionally:

• Consider masking permission bits (e.g. mode & 0777) to prevent setuid/setgid/sticky bits being set via RPC.
• Consider enforcing maximum read/write sizes per request/stream to avoid disk exhaustion DoS inside the VM.

6. 🟠 SSRF bypass in Playwright browser URL validation (numeric/IPv6 forms, DNS/redirect gaps)

Description

validateBrowserURL() is used as SSRF protection for Docker/gVisor browser automation (ExecBrowserHelper.navigateBrowser()), but it only performs string-based hostname checks and does not validate how Chromium resolves/normalizes hosts at request time.

As a result, an attacker who can control the navigated URL can reach internal/metadata endpoints from the container network (default bridge) via multiple bypass classes:

• Alternative IP encodings that Chromium treats as loopback/private, but the validator does not detect:
  • http://2130706433/ (decimal IPv4 for 127.0.0.1)
  • http://0x7f000001/ (hex IPv4 for 127.0.0.1)
  • http://[::ffff:127.0.0.1]/ (IPv4-mapped IPv6 loopback)
• IPv6 private ranges not covered (e.g., ULA fc00::/7, link-local fe80::/10) and other non-public IPv4 ranges not covered.
• DNS resolution/rebinding: any attacker-controlled hostname that resolves to a private IP (or later rebinds) will pass the check because no DNS lookup is performed.
• Redirect-to-private: even if the initial URL is public and passes validation, a remote server can return a redirect (3xx) to a private/loopback/metadata URL; the code validates only the initial URL and does not enforce a policy on subsequent requests.

This is particularly impactful because sandbox containers default to --network=bridge, so the browser has outbound network access and can typically reach cloud metadata IPs and internal services.

Recommendation

Treat URL string validation as defense-in-depth only; enforce SSRF protection at the network/request layer.

Code-level mitigations (recommended combined):

1. Resolve and validate all destination IPs before navigation (and ideally before every request):

   • Perform dns.lookup(host, { all: true }) and reject if any A/AAAA is non-public.
   • Normalize and check IPv6 (including IPv4-mapped IPv6) and block all non-public ranges: RFC1918, loopback, link-local, ULA (fc00::/7), multicast, reserved, etc.

2. Block redirects and subresource requests to private ranges using Playwright routing so that redirects cannot bypass the initial check:

import dns from "node:dns/promises";
import ipaddr from "ipaddr.js";

function isPublicIP(ipStr: string): boolean {
  const ip = ipaddr.parse(ipStr);
  const normalized = ip.kind() === "ipv6" && ip.isIPv4MappedAddress()
    ? ip.toIPv4Address()
    : ip;
  const range = normalized.range();
  ​// ipaddr.js ranges: 'private', 'loopback', 'linkLocal', 'uniqueLocal', etc.
  return range === "unicast"; // adjust to allow only true public unicast
}

export async function validateBrowserURLStrict(rawURL: string) {
  const u = new URL(rawURL);
  if (!(["http:", "https:"].includes(u.protocol))) throw new Error("unsupported protocol");

  const host = u.hostname;

  ​// If host is a literal IP, validate directly
  if (net.isIP(host)) {
    if (!isPublicIP(host)) throw new Error("blocked address");
    return;
  }

  ​// Resolve DNS and validate all records
  const addrs = await dns.lookup(host, { all: true, verbatim: true });
  if (addrs.length === 0) throw new Error("unresolvable host");
  for (const a of addrs) {
    if (!isPublicIP(a.address)) throw new Error(`blocked address: ${a.address}`);
  }
}

​// In connectScript body, before page.goto:
await page.route('**/*', async (route) => {
  try {
    await validateBrowserURLStrict(route.request().url());
    await route.continue();
  } catch {
    await route.abort();
  }
});

3. Safer defaults: consider defaulting browser sandboxes to a more restrictive egress posture (e.g., --network=none + explicit proxy/allowlist), or at minimum implement host-level/container-level egress firewall rules to block private/metadata ranges (this is the only robust mitigation against DNS rebinding).

Also add regression tests for the bypass cases above (decimal/hex IPv4, IPv4-mapped IPv6, redirects).

7. 🟠 Unauthenticated VNC WebSocket proxy and expvar endpoint exposed on all interfaces by default

Description

The vm-runner starts an HTTP server for the VNC WebSocket proxy and Go expvar metrics on a TCP port that binds to all interfaces and has no authentication.

• DefaultServiceConfig() enables VNCProxyPort by default (6080).
• http.ListenAndServe(":<port>", mux) binds to 0.0.0.0:<port> (and ::<port>), making the endpoints remotely reachable if the host is network-accessible.
• The mux exposes:
  • /vnc → a WebSocket bridge into sandbox VNC sessions (powerful interactive control surface)
  • /debug/vars → expvar.Handler() (leaks process info such as command line, memstats, and custom metrics)

Vulnerable code:

httpMux.HandleFunc("/vnc", vncProxy.HandleWS)
httpMux.Handle("/debug/vars", expvar.Handler())
...
addr := fmt.Sprintf(":%d", cfg.VNCProxyPort)
http.ListenAndServe(addr, httpMux)

Even if VNC inside the guest uses a password, exposing these endpoints on a network port without any access control significantly increases remote attack surface (information disclosure + potential unauthorized interactive access if other defenses fail/misconfigure).

Recommendation

Harden the HTTP listener:

1. Disable by default (set default VNCProxyPort to 0) and require explicit opt-in.
2. Bind to loopback unless explicitly configured otherwise.
3. Add authentication/authorization (e.g., per-sandbox bearer token) and consider TLS termination.
4. Remove or protect /debug/vars behind auth.

Example (bind loopback and require explicit enable):

​// Default: disabled
cfg.VNCProxyPort = 0

​// When enabled, bind loopback only
addr := fmt.Sprintf("127.0.0.1:%d", cfg.VNCProxyPort)
log.Printf("VNC WebSocket proxy on %s", addr)
if err := http.ListenAndServe(addr, httpMux); err != nil && err != http.ErrServerClosed {
    ...
}

And gate /debug/vars:

httpMux.Handle("/debug/vars", requireAuth(expvar.Handler()))

8. 🟠 Symlink-following file write in chroot preparation can overwrite arbitrary host files

Description

The chroot preparation path falls back from hard-linking to copying via copyFile(). The copy implementation uses os.Create(dst), which follows symlinks.

If an attacker can pre-create dst (e.g., .../root/<kernel basename> or .../root/<rootfs basename>) as a symlink to an arbitrary host path, the fallback copy will:

• follow the symlink
• truncate/overwrite the symlink target on the host as the vm-runner user (often root)

This is especially risky because linkOrCopy() falls back to copyFile() on any os.Link error (including EEXIST when a symlink/file already exists), making the symlink pre-creation path sufficient to trigger the vulnerable write.

Vulnerable code:

func linkOrCopy(src, dst string) error {
    if err := os.Link(src, dst); err == nil {
        return nil
    }
    return copyFile(src, dst)
}

func copyFile(src, dst string) error {
    ...
    out, err := os.Create(dst) // follows symlinks
    ...
}

Recommendation

Harden destination creation to avoid following symlinks and avoid copying over pre-existing paths.

Minimum fixes:

1. Do not fall back to copy on all os.Link errors; only fall back for the expected cross-device case (EXDEV). Treat EEXIST as an error.
2. Create the destination securely using O_NOFOLLOW + O_EXCL (or equivalent) and/or verify with Lstat that the destination does not exist and is not a symlink.

Example (Linux) using golang.org/x/sys/unix:

import "golang.org/x/sys/unix"

func safeCreateNoFollow(dst string, perm os.FileMode) (*os.File, error) {
	fd, err := unix.Open(dst, unix.O_WRONLY|unix.O_CREAT|unix.O_EXCL|unix.O_NOFOLLOW, uint32(perm))
	if err != nil {
		return nil, err
	}
	return os.NewFile(uintptr(fd), dst), nil
}

func linkOrCopy(src, dst string) error {
	if err := os.Link(src, dst); err == nil {
		return nil
	} else if errors.Is(err, unix.EXDEV) {
		return copyFileNoFollow(src, dst)
	} else {
		return err
	}
}

func copyFileNoFollow(src, dst string) error {
	in, err := os.Open(src)
	if err != nil { return err }
	defer in.Close()

	out, err := safeCreateNoFollow(dst, 0644)
	if err != nil { return err }
	defer out.Close()

	_, err = io.Copy(out, in)
	return err
}

Also consider removing any pre-existing VM chroot directory before preparing it (or creating it under a new random name) to reduce the attack surface from stale files/symlinks.

9. 🟡 Memory exhaustion DoS when hashing rootfs via os.ReadFile during snapshot metadata writing

Description

writeMetadata() reads the entire rootfs image into memory to compute RootfsHash:

• Rootfs images in this project are ext4 disk images (see ServiceConfig.RootfsPath comment and images/rootfs/build-rootfs.sh which creates 256–600MB+ ext4 images).
• Snapshotter.Create() calls writeMetadata() on every snapshot creation. The warm snapshot pool (internal/vm/pool.go) creates snapshots in parallel during startup warm-up and replenishment.
• os.ReadFile() loads the full rootfs into RAM; with multiple concurrent snapshot creations, this can spike memory and crash the service (DoS).

Vulnerable code:

rootfsData, err := os.ReadFile(cfg.RootfsPath)
...
rootfsHash := hashBytes(rootfsData)

While today snapshot creation appears to be triggered by the runner itself (pool warmup), the rootfs path is also overrideable via gRPC CreateSandbox (req.rootfs_path), increasing the risk if snapshotting becomes exposed or reused for non-golden VMs.

Recommendation

Avoid reading the entire rootfs into memory; stream it into the hash function.

Example fix (streaming SHA256):

func hashFileSHA256(path string) (string, error) {
	f, err := os.Open(path)
	if err != nil { return "", err }
	defer f.Close()

	h := sha256.New()
	if _, err := io.Copy(h, f); err != nil {
		return "", err
	}
	return hex.EncodeToString(h.Sum(nil)), nil
}

func writeMetadata(dir string, artifacts *SnapshotArtifacts, cfg *VMConfig) error {
	rootfsHash, err := hashFileSHA256(cfg.RootfsPath)
	if err != nil {
		rootfsHash, err = hashFileSHA256(artifacts.RootfsPath)
		if err != nil { return fmt.Errorf("hash rootfs for metadata: %w", err) }
	}
	​// ... config hash stays as-is (small JSON)
	​// ...
}

Optionally:

• Add an upper bound on acceptable rootfs size (e.g., stat the file and refuse >N GiB) before hashing.
• Reuse the already-streamed rootfs hashing approach used in computeVersion() to avoid duplicated work.

10. 🟡 Shell command injection via argv joining and sh -lc in Exec bridge

Description

The host-side ExecService accepts repeated string command (argv-like) but converts it into a single shell string and executes it via sh -lc in the guest.

• Input: StartExec.command[] comes from the gRPC client (potentially influenced by user-controlled data).
• Unsafe transformation: strings.Join(args, " ") drops quoting/escaping semantics.
• Sink: envd executes the resulting string with exec.CommandContext(ctx, "sh", "-lc", command).

This enables shell metacharacter injection even when the caller intended to pass data as an argument.

Example: command = ["echo", "x; rm -rf /workspace"] becomes echo x; rm -rf /workspace and the rm executes in the VM.

Vulnerable code (host bridge and guest execution):

​// host: argv -> shell string
shellCmd := strings.Join(startExec.GetCommand(), " ")

​// guest: executes through a shell
cmd := exec.CommandContext(ctx, "sh", "-lc", req.GetCommand())

Recommendation

Avoid invoking a shell when the API is argv-based.

Preferred fix (change envd API to argv):

1. Update envd.v1.StartRequest to carry an argv array (e.g., repeated string argv = 1;).
2. Execute directly without sh -lc:

​// guest envd: direct exec
argv := req.GetArgv()
if len(argv) == 0 {
  return status.Error(codes.InvalidArgument, "argv required")
}
cmd := exec.CommandContext(ctx, argv[0], argv[1:]...)

If you must keep shell support:

• Make it explicit (e.g., StartRequest.shell_command), and do not accept repeated string command as argv.
• Or escape each argument with a robust shell-escaping library before joining (still less safe than direct exec).

Also consider enforcing strong access controls on the host gRPC Unix socket (e.g., chmod 0600) since this is a powerful primitive.

11. 🟡 envd optional TCP listener exposes unauthenticated privileged gRPC API to any local VM process

Description

The envd guest agent can be started with --tcp, which binds its gRPC server to localhost:<port> with no authentication or authorization.

If enabled (accidentally or via misconfiguration), any unprivileged process running inside the MicroVM (e.g., the sandbox user, a compromised browser process, etc.) can connect to 127.0.0.1:<port> and invoke highly privileged RPCs, including:

• Arbitrary command execution via ProcessService.Start (sh -lc <command>)
• Arbitrary filesystem read/write/delete via FileService.ReadFile / WriteFile / Remove

Because OpenRC services run as root by default (and the init script does not set command_user), envd will typically run as root, so these RPCs can become a local privilege escalation to root within the VM.

Vulnerable code (enables localhost TCP listener):

useTCP := flag.Bool("tcp", false, "Listen on TCP localhost:50051 instead of vsock (for local development)")
...
addr := fmt.Sprintf("localhost:%d", *port)
lis, err = net.Listen("tcp", addr)
...
if err := srv.GRPC.Serve(lis); err != nil { ... }

Note: the provided OpenRC script currently starts /usr/local/bin/envd without --tcp, so the issue is specifically the presence of this insecure mode which can be enabled at runtime.

Recommendation

Prevent untrusted local VM processes from accessing privileged envd RPCs.

Recommended options (defense-in-depth):

1. Remove or hard-disable TCP mode in production builds (e.g., build tags / separate dev-only binary):

​//go:build dev
useTCP := flag.Bool("tcp", false, "...")

2. Add authentication/authorization at the gRPC layer (for both vsock and TCP), e.g. require a per-VM shared token in gRPC metadata:

import "google.golang.org/grpc/metadata"

func authUnaryInterceptor(token string) grpc.UnaryServerInterceptor {
  return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) {
    md, _ := metadata.FromIncomingContext(ctx)
    if len(md.Get("x-envd-token")) != 1 || md.Get("x-envd-token")[0] != token {
      return nil, status.Error(codes.Unauthenticated, "missing/invalid token")
    }
    return handler(ctx, req)
  }
}

s := grpc.NewServer(grpc.UnaryInterceptor(authUnaryInterceptor(token)))

Store the token in a root-only file or pass it via a protected channel at boot.

3. Run envd with reduced privileges (e.g., set command_user="sandbox" in OpenRC, and/or drop capabilities) and explicitly run executed commands as a less-privileged user where possible.

4. If TCP is kept for development, consider binding to a Unix domain socket with restrictive filesystem permissions instead of TCP, so only authorized users/processes can connect.

12. 🟡 Race condition allows eviction to delete snapshot artifacts while they are being restored (DoS risk)

Description

The snapshot pool eviction logic can delete snapshot directories that have been acquired for restore but are not in the ready queue anymore.

• Manager.Create() acquires a snapshot via Pool.Acquire() and immediately starts restoring it from snapDir.
• Concurrently, the pool background replenishLoop() calls evictIfOverLimit() when len(p.ready) < p.size.
• evictIfOverLimit() only protects directories currently present in the p.ready channel (by draining it into readySet).
• An acquired snapshot is removed from p.ready and becomes unprotected, so eviction may call os.RemoveAll() on that directory while restore is still opening/reading memory.bin and vmstate.snap.

Impact:

• Snapshot restore can fail intermittently (missing artifacts), forcing the system into expensive cold boots.
• Under load (many sandbox create requests) + disk limit pressure, this can be abused to cause sustained performance degradation / resource exhaustion (denial of service).

Vulnerable code (eviction removes any non-readySet directory):

for _, d := range dirs {
    if totalSize <= p.diskLimitBytes {
        break
    }
    if readySet[d.path] {
        continue // never evict ready snapshots
    }
    if err := os.RemoveAll(d.path); err != nil {
        ...
    }
    totalSize -= d.size
}

Recommendation

Prevent eviction of in-flight snapshots (acquired but not yet fully restored).

A straightforward approach is to explicitly track leased snapshots:

• Add a leased set protected by a mutex.
• Acquire adds the path to leased before returning it.
• The restore path (e.g. Manager.trySnapshotRestore) removes it from leased after restore completes (success or failure).
• evictIfOverLimit must skip both ready and leased paths.

Example sketch:

​// in Pool
leasedMu sync.Mutex
leased   map[string]struct{}

func (p *Pool) Acquire(ctx context.Context) (string, func(), error) {
    select {
    case path := <-p.ready:
        p.leasedMu.Lock()
        p.leased[path] = struct{}{}
        p.leasedMu.Unlock()

        release := func() {
            p.leasedMu.Lock()
            delete(p.leased, path)
            p.leasedMu.Unlock()
        }
        return path, release, nil
    case <-ctx.Done():
        return "", nil, ctx.Err()
    }
}

​// in evictIfOverLimit
p.leasedMu.Lock()
_, isLeased := p.leased[d.path]
p.leasedMu.Unlock()
if isLeased || readySet[d.path] {
    continue
}

Alternatively, a simpler (but more blocking) fix is to reuse a single mutex so Acquire and evictIfOverLimit cannot overlap (at the cost of higher latency while eviction scans the filesystem).

13. 🔵 Unvalidated snapshot metadata allows integer wrap and unsafe resource configuration during restore

Description

The snapshot restore path trusts metadata.json values for CPU and memory sizing without validating ranges or sign, then casts int64 to uint32.

If an attacker can modify snapshot artifacts on disk (e.g., --snapshot-dir points to a location writable by other users, or permissions/ownership are misconfigured), they can set negative or excessively large values in metadata.json:

• meta.VcpuCount / meta.MemSizeMib are int64 and can be negative
• these are cast to uint32 (uint32(-1) becomes 4294967295)
• the resulting values flow into Firecracker configuration and jailer cgroup limits, potentially causing DoS, unexpected resource allocation attempts, or restore failures/crashes

Vulnerable code:

req := &CreateRequest{
    SandboxID:  newSandboxID,
    VcpuCount:  uint32(meta.VcpuCount),
    MemSizeMib: uint32(meta.MemSizeMib),
}
vmCfg := &VMConfig{
    ​// ...
    VcpuCount:  meta.VcpuCount,
    MemSizeMib: meta.MemSizeMib,
}

Note: readMetadata() performs JSON unmarshal only; there is no validation that these values are positive, within service policy, or even within uint32 bounds before use.

Recommendation

Validate and constrain snapshot metadata before using it to size the restored VM.

• Reject negative values
• Reject values exceeding math.MaxUint32
• Enforce service-level min/max policy (e.g., 1 <= vcpu <= 32, 128 <= memMiB <= 65536)
• Optionally ensure snapshot artifacts are trusted (e.g., enforce snapshot dir ownership/permissions, or authenticate metadata via signature/HMAC).

Example hardening in RestoreEntry:

import "math"

const (
  maxVcpu = int64(32)
  maxMem  = int64(65536) // 64 GiB
)

if meta.VcpuCount < 1 || meta.VcpuCount > maxVcpu {
  return nil, fmt.Errorf("invalid snapshot vcpu_count: %d", meta.VcpuCount)
}
if meta.MemSizeMib < 128 || meta.MemSizeMib > maxMem {
  return nil, fmt.Errorf("invalid snapshot mem_size_mib: %d", meta.MemSizeMib)
}
if meta.VcpuCount > int64(math.MaxUint32) || meta.MemSizeMib > int64(math.MaxUint32) {
  return nil, fmt.Errorf("snapshot resources exceed uint32")
}

req.VcpuCount = uint32(meta.VcpuCount)
req.MemSizeMib = uint32(meta.MemSizeMib)

Also consider writing snapshot dirs with stricter permissions (e.g., 0700) and verifying the snapshot base dir is not group/world-writable at startup.

14. 🔵 Playwright stdout marker collision allows untrusted page content to break result parsing (DoS)

Description

ExecBrowserHelper.parseResult() frames JSON results by searching stdout for fixed markers using indexOf(). If the JSON payload contains the end marker substring (---PW_END---)—which is plausible when returning page-derived strings like textContent(), innerHTML(), page.title(), or String(page.evaluate(...))—endIdx will match the marker inside the JSON string and truncate the JSON.

Impacts:

• Remote DoS / reliability failure: navigating to or extracting content from an attacker-controlled page that includes ---PW_END--- causes JSON.parse() to throw and the operation to fail.
• Potential log amplification/data leak: on parse failure the code throws an error including raw stdout, which can include large attacker-controlled HTML/text.

Vulnerable code:

const startIdx = stdout.indexOf(RESULT_START);
const endIdx = stdout.indexOf(RESULT_END);
...
jsonStr = stdout.substring(startIdx + RESULT_START.length, endIdx).trim();
...
throw new Error(`Failed to parse Playwright result. Raw stdout: ${stdout}`);

Marker injection is plausible because output includes JSON.stringify(__result) and __result can include untrusted page content:

const output = "---PW_RESULT---" + "\n" + JSON.stringify(__result) + "\n" + "---PW_END---";

Recommendation

Use a framing method that cannot be confused by payload content.

Option A (minimal change): use the last end marker and validate ordering:

const startIdx = stdout.indexOf(RESULT_START);
const endIdx = stdout.lastIndexOf(RESULT_END);
if (startIdx !== -1 && endIdx !== -1 && endIdx > startIdx) {
  jsonStr = stdout.substring(startIdx + RESULT_START.length, endIdx).trim();
} else {
  jsonStr = stdout.trim();
}

Option B (more robust): length-prefix or base64 the JSON payload:

const json = JSON.stringify(__result);
const b64 = Buffer.from(json, 'utf8').toString('base64');
process.stdout.write(`${RESULT_START}\n${b64}\n${RESULT_END}`);

​// decode base64 then JSON.parse

Also avoid embedding full raw stdout in thrown errors; truncate and/or sanitize (e.g., first/last 4KB).

Analyzed PR: #41437 at commit f27fc2e

_{Last updated on: 2026-03-09T23:16:58Z}

@vincentkoc @steipete @altaywtf — Requesting review for this pluggable sandbox provider architecture.

What this PR adds:

• ISandboxProvider interface enabling 3-tier isolation: Docker (namespaces) → gVisor (user-space kernel) → Firecracker (KVM hardware virtualization)
• Auto-detection with fallback chain: tries Firecracker first, then gVisor, then Docker
• IBrowserCapable interface for browser automation across all three backends
• Go-based openclaw-vm-runner for Firecracker MicroVM lifecycle management with snapshot pool (~3ms restore)

Security improvements:

• Hardware-level KVM isolation via Firecracker — full VM boundary, not just container namespaces
• gVisor syscall filtering as a middle-ground option (stronger than Docker, lighter than Firecracker)
• SSRF protection with URL validation blocking private IPs, cloud metadata endpoints, and non-HTTP protocols
• Secret filtering for environment variables passed to sandboxes
• Resource limits and network isolation configurable per-sandbox

Why this matters:

• Current Docker-only sandbox has a single isolation layer. This PR adds defense-in-depth with up to 3 independent isolation boundaries
• Snapshot pooling opens the path to sub-10ms sandbox cold starts, critical for interactive agent workflows
• The pluggable architecture makes it straightforward to add future backends without touching existing code

@vincentkoc — You merged our previous sandbox PR #29974, so you have context on this work's direction.

Happy to address any feedback. All CI checks pass except secrets (upstream pnpm-audit-prod CVEs, not introduced by this PR).

Please don’t spam-ping multiple maintainers at once. Be patient, or join our community Discord for help: https://discord.gg/clawd

@aisle-research-bot review