Ray Sandboxing with gVisor (experimental)

[andrewsykim]

Description

This PR adds the first version of the experimental sandboxing library in Ray based on gVisor. See the REP for more details.

This PR was tested with the following example scripts:

• https://gist.github.com/andrewsykim/7286c607b7523435303102bf8517d23b
• https://gist.github.com/andrewsykim/00d1cb20b63b821c725c71eaa231fb13
• https://gist.github.com/andrewsykim/3c82d1b97c02ee71f1cc27cbd2b0f1ed

Related issues

Link related issues: "Fixes #1234", "Closes #1234", or "Related to #1234".

Additional information

Optional: Add implementation details, API changes, usage examples, screenshots, etc.

[pcmoritz]

Thanks a lot for putting this together it looks great as a first cut! I'm curious, in the K8s setups you are aware of, would the pods have permissions to create pods (that seems like the main prerequisite of this PR).

[andrewsykim]

yes, that's correct. The user needs to grant the service account mounted to the Pod access to create Pods. It'll be up to users to lock down the access to specific namespaces where they expect sandboxes to run.

[andrewsykim]

I updated this POC to also include gVisor as a sandbox runtime. When using gVisor, Ray will invoke runsc (OCI runtime from gVisor) to start containers inside the Ray node.

[pcmoritz]

While trying the PR out, I ran into this. I think instead of saying "on the node" here, it would be better to say "on the container", since it is far far likely that Ray runs in a container (or you can say "on the container or node").

Are there any plans to get runsc into the official ubuntu / debian archives? That would make it much less of a hassle to install and make it more likely for people to adopt it.

Also, after installing runsc, I ran into gVisor container failed to start: running container: creating container: creating container root directory "/var/run/runsc": mkdir /var/run/runsc: permission denied

In containers, it seems quite likely that /var/run is not writable, maybe we can set a configuration option to use a different directory like /tmp?

[pcmoritz]

If I add run_args.extend(["--root", "/tmp/runsc"]) to the run args, I can work around this error (and --root also needs to be passed in when a command is executed)

[pcmoritz]

This is the patch I ended up with to get it working in the container:

--- a/python/ray/experimental/sandbox/backend/gvisor.py
+++ b/python/ray/experimental/sandbox/backend/gvisor.py
@@ -23,6 +23,10 @@
 
 logger = logging.getLogger(__name__)
 
+# Directory where runsc keeps container state. Every runsc invocation for a
+# sandbox must agree on this, otherwise the container cannot be looked up.
+_RUNSC_ROOT = "/tmp/runsc"
+
 
 def _call_actor(func, *args, **kwargs):
     try:
@@ -78,9 +82,7 @@
                 cpu=config.cpu,
                 memory=config.memory,
             )
-            run_args = [runsc_path]
-            if config.rootless:
-                run_args.append("--rootless")
+            run_args = self._runsc_base_args(runsc_path, config)
             if config.network:
                 run_args.extend(["--network", config.network])
             run_args.extend(["run", "--bundle", root_dir, sandbox_id])
@@ -117,9 +119,7 @@
             proc = meta.get("proc")
 
             if self._runsc_path_override is None:
-                kill_args = [runsc_path]
-                if config.rootless:
-                    kill_args.append("--rootless")
+                kill_args = self._runsc_base_args(runsc_path, config)
                 kill_args.extend(["kill", sandbox_id, "SIGKILL"])
                 subprocess.run(kill_args, capture_output=True)
 
@@ -130,9 +130,7 @@
                     except subprocess.TimeoutExpired:
                         proc.kill()
 
-                del_args = [runsc_path]
-                if config.rootless:
-                    del_args.append("--rootless")
+                del_args = self._runsc_base_args(runsc_path, config)
                 del_args.extend(["delete", sandbox_id])
                 subprocess.run(del_args, capture_output=True)
 
@@ -203,9 +201,7 @@
 
         wrapped_cmd = f"({cmd_str}) > '{container_out}' 2> '{container_err}'"
 
-        runsc_args = [runsc_path]
-        if config.rootless:
-            runsc_args.append("--rootless")
+        runsc_args = self._runsc_base_args(runsc_path, config)
         runsc_args.extend(["exec", "-cwd", raw_cwd])
         if env:
             for k, v in env.items():
@@ -290,6 +286,14 @@
             return SandboxStatus.RUNNING
         return SandboxStatus.TERMINATED
 
+    def _runsc_base_args(self, runsc_path: str, config: SandboxConfig) -> List[str]:
+        """Build the runsc global flags shared by run/exec/kill/delete."""
+        args = [runsc_path]
+        if config.rootless:
+            args.append("--rootless")
+        args.extend(["--root", _RUNSC_ROOT])
+        return args
+
     def _resolve_path(self, root_dir: str, relative_or_abs_path: str) -> str:
         clean_path = relative_or_abs_path.lstrip("/")
         return os.path.join(root_dir, clean_path)

[abrarsheikh]

i assume this stripping of / is to restrict host access, then someone could do ../../<path> to gain access to the host FS?

are we going through runsc for file access?

[andrewsykim]

are we going through runsc for file access?

yes

[pcmoritz]

We should remove the default here and anywhere else -- otherwise we will need to continuously upgrade it when new versions get released (3.10 is very old and any version we put as a default will get outdated) and that's not ok behavior for users. Instead it is better to have it in the docs / example to make it easy to get started.

[andrewsykim]

yup noted, I will remove this, the image is not actually used anywhere in the code yet anyways

[pcmoritz]

I'm skeptical that trying to wrap this in SandboxHandle is the right thing to do -- it makes it much harder to orchestrate multiple sandboxes -- most of the calls are wrapped in ray.get so they are blocking. So the only way to use multiple of them is via the async methods and you can't do that in a sync context / through the Ray API in an idiomatic Ray program.

[andrewsykim]

@pcmoritz looking for your input here on how to handle default bindings (read only) when images are not provided.

It seems kind of hacky to include something like /home/ray/anaconda3/bin, but if you look at the default Ray images, this is where the Python binaries live. If we don't set this, then there's no Python executable available in the image unless a user creates a custom image where Python executable is in one of the default binds (/bin, /usr, /lib/, etc)

[pcmoritz]

The most important settings is the setting where the image is provided, and in that case by default we should not bind mount anything or set additional env variables, just what is in the image (and make it possible for the user to add bind mounts or environment variables via the config options).

If somebody really wants to run without the image, I think by default we should just not bind mount anything or set any env variables by default and let the user configure it. That is both the safest default (what would be expected from a "sandbox") and also doesn't require us to hardcode or assume anything about the file system layout of the outer layer. But this scenario is far less important than the image case for the RL use cases we are targeting -- almost anything will run with container images.

The second case is kind of like https://github.com/containers/bubblewrap and I think people like the bubblewrap defaults. Now there is the additional complication that this might differ from the gvisor defaults -- if that's the case, we could always require an image and only allow running without an image if the gvisor configs are explicitly set? I do want to avoid (a) having ad hoc defaults like anaconda hard coded in the code and (b) having defaults that are unsafe like exposing the whole host filesystem -- we should only do unsafe things if the user explicitly asks us to do them.

Let me know what you think / if you think we should do something different :)

[andrewsykim]

I am leaning towards requiring an image and not doing any default bind mounts, except for the user specified working dir. I think this is significantly simpler for now and easier to reason about whether something is readonly or not. We may get users asking about sandboxes without images but we can revisit if we get user feedback, we know for sure that useres will want to use images

[gemini-code-assist]

Code Review

This pull request introduces a new experimental sandboxing feature for Ray using gVisor (runsc), allowing isolated execution of commands inside container environments. It includes an OCI image puller and extractor, a gVisor backend, a Ray actor wrapper, and comprehensive unit tests. The review feedback highlights several critical issues: a path traversal vulnerability on Windows due to platform-dependent path splitting; potential Out-Of-Memory (OOM) crashes when loading large container images and layers entirely into memory; a potential deadlock and resource leaks in the sandbox creation process; and an unimplemented ttl_seconds cleanup mechanism.

[gemini-code-assist]

The path traversal check splits the path using os.sep. On Windows or in environments with mixed separators, os.sep is \\ while tar files always use / as the separator. This allows malicious tar files to bypass the check and perform path traversal attacks. Normalize the separators to / and split on / to ensure platform-independent security.

Suggested change

	# Prevent path traversal
	if ".." in name.split(os.sep) or name.startswith(os.sep):
	continue
	# Prevent path traversal
	normalized_name = name.replace("\\", "/")
	parts = normalized_name.split("/")
	if ".." in parts or normalized_name.startswith("/"):
	continue

[gemini-code-assist]

extract_tar_layer currently accepts tar_bytes: bytes and loads the entire archive into memory. For large container images, this can easily cause Out-Of-Memory (OOM) crashes. Refactor extract_tar_layer to accept either bytes or a file path (str), allowing us to process large tar files directly from disk without loading them into memory.

def extract_tar_layer(tar_source: Union[bytes, str], dest_dir: str) -> None:
    """Extract a tar archive layer onto dest_dir with OCI whiteout handling."""
    tar_context = (
        tarfile.open(fileobj=io.BytesIO(tar_source), mode="r:*")
        if isinstance(tar_source, bytes)
        else tarfile.open(name=tar_source, mode="r:*")
    )
    with tar_context as tar:

[gemini-code-assist]

Avoid reading the entire local or cached tar archive into memory. Instead, pass the file path directly to the refactored extract_tar_layer function to stream the extraction from disk.

            if os.path.isfile(image):
                try:
                    extract_tar_layer(image, tmp_extract_dir)
                except Exception as err:
                    shutil.rmtree(tmp_extract_dir, ignore_errors=True)
                    raise SandboxCreationError(
                        f"Failed to extract local image archive '{image}': {err}"
                    ) from err
            elif os.path.isfile(tar_path):
                try:
                    extract_tar_layer(tar_path, tmp_extract_dir)
                except Exception as err:
                    shutil.rmtree(tmp_extract_dir, ignore_errors=True)
                    raise SandboxCreationError(
                        f"Failed to extract cached image archive '{tar_path}': {err}"
                    ) from err

[gemini-code-assist]

Avoid loading the entire remote layer blob into memory via blob_resp.read(). Stream the response directly to a temporary file on disk and extract it from there to prevent OOM crashes during image pulling.

Suggested change

	for layer in layers:
	digest = layer["digest"]
	blob_url = f"https://{registry}/v2/{repo}/blobs/{digest}"
	blob_req = urllib.request.Request(blob_url, headers=headers)
	with urllib.request.urlopen(
	blob_req, timeout=timeout_seconds
	) as blob_resp:
	layer_bytes = blob_resp.read()
	extract_tar_layer(layer_bytes, tmp_extract_dir)
	for layer in layers:
	digest = layer["digest"]
	blob_url = f"https://{registry}/v2/{repo}/blobs/{digest}"
	blob_req = urllib.request.Request(blob_url, headers=headers)
	temp_layer_path = os.path.join(images_dir, f"{safe_name}.layer.{uuid.uuid4().hex}.tmp")
	try:
	with urllib.request.urlopen(blob_req, timeout=timeout_seconds) as blob_resp, open(temp_layer_path, "wb") as f_out:
	shutil.copyfileobj(blob_resp, f_out)
	extract_tar_layer(temp_layer_path, tmp_extract_dir)
	finally:
	if os.path.exists(temp_layer_path):
	try:
	os.remove(temp_layer_path)
	except OSError:
	pass

[gemini-code-assist]

This method has two major issues:

1. If an exception or timeout occurs during sandbox creation, the local directory structure, runsc container state, and started processes are leaked on the node.
2. Running runsc run with stdout=subprocess.PIPE and stderr=subprocess.PIPE without reading from them concurrently can cause a deadlock if the buffer fills up (64KB limit).

Refactor the method to redirect runsc output to a log file (which also aids debugging) and wrap the creation process in a try...except block to clean up all resources on failure.

    def create_sandbox(self, config: SandboxConfig) -> str:
        """Create a local directory structure and initialize a gVisor sandbox instance."""
        if not shutil.which("runsc"):
            raise SandboxCreationError(
                "gVisor executable 'runsc' not found in PATH. "
                "Please install gVisor (runsc) on the node."
            )

        sandbox_uuid = uuid.uuid4().hex[:12]
        sandbox_id = f"ray-sandbox-{sandbox_uuid}"
        root_dir = os.path.join(_RAY_SANDBOX_DIR, sandbox_id)

        try:
            os.makedirs(root_dir, mode=0o777, exist_ok=True)
            work_dir_path = os.path.abspath(
                os.path.join(root_dir, config.work_dir.lstrip("/"))
            )
            if not (
                work_dir_path == os.path.abspath(root_dir)
                or work_dir_path.startswith(os.path.abspath(root_dir) + os.sep)
            ):
                raise SandboxCreationError(
                    f"Invalid work_dir '{config.work_dir}': Path traversal detected."
                )
            os.makedirs(work_dir_path, mode=0o777, exist_ok=True)
        except Exception as err:
            if os.path.exists(root_dir):
                shutil.rmtree(root_dir, ignore_errors=True)
            raise SandboxCreationError(
                f"Failed to initialize local sandbox directory '{root_dir}': {err}"
            ) from err

        proc = None
        try:
            # Prepare OCI bundle config for long-running container process
            self._prepare_oci_bundle(
                root_dir=root_dir,
                work_dir_path=work_dir_path,
                container_cwd=config.work_dir,
                image=config.image,
                env_dict=config.env,
                cpu=config.cpu,
                memory=config.memory,
                readonly=config.readonly,
            )
            run_args = self._runsc_base_args(config)
            if config.network:
                run_args.extend(["--network", config.network])
            overlay_dir = os.path.join(root_dir, "overlay")
            os.makedirs(overlay_dir, mode=0o777, exist_ok=True)
            run_args.append(f"--overlay2=root:dir={overlay_dir}")
            run_args.extend(["run", "--bundle", root_dir, sandbox_id])

            log_file_path = os.path.join(root_dir, "runsc.log")
            with open(log_file_path, "w", encoding="utf-8") as log_file:
                proc = subprocess.Popen(
                    run_args,
                    stdin=subprocess.DEVNULL,
                    stdout=log_file,
                    stderr=log_file,
                )
            start_time = time.time()
            timeout = config.timeout_seconds
            state_args = self._runsc_base_args(config) + ["state", sandbox_id]

            while True:
                if proc.poll() is not None:
                    stderr_str = ""
                    if os.path.exists(log_file_path):
                        try:
                            with open(log_file_path, "r", encoding="utf-8", errors="replace") as f:
                                stderr_str = f.read()
                        except Exception:
                            pass
                    raise SandboxCreationError(
                        f"gVisor container failed to start: {stderr_str}"
                    )

                res = subprocess.run(state_args, capture_output=True, text=True)
                if res.returncode == 0:
                    try:
                        state_data = json.loads(res.stdout)
                        if state_data.get("status") == "running":
                            break
                    except Exception:
                        pass

                if time.time() - start_time > timeout:
                    raise SandboxTimeoutError(
                        f"gVisor container '{sandbox_id}' failed to reach 'running' state within {timeout} seconds."
                    )

                time.sleep(0.1)

            self._sandbox_metadata[sandbox_id] = {
                "root_dir": root_dir,
                "work_dir": work_dir_path,
                "config": config,
                "proc": proc,
                "status": SandboxStatus.RUNNING,
            }
            return sandbox_id

        except Exception as err:
            # Cleanup on failure
            kill_args = self._runsc_base_args(config)
            kill_args.extend(["kill", sandbox_id, "SIGKILL"])
            subprocess.run(kill_args, capture_output=True)

            if proc:
                if proc.poll() is None:
                    proc.terminate()
                    try:
                        proc.communicate(timeout=2)
                    except subprocess.TimeoutExpired:
                        proc.kill()

            del_args = self._runsc_base_args(config)
            del_args.extend(["delete", sandbox_id])
            subprocess.run(del_args, capture_output=True)

            shutil.rmtree(root_dir, ignore_errors=True)
            raise

[gemini-code-assist]

Import Union from typing to support type annotations for the refactored extract_tar_layer function.

Suggested change

	from typing import Dict, Tuple
	from typing import Dict, Tuple, Union

[gemini-code-assist]

ttl_seconds is defined in the configuration but is currently completely ignored and unimplemented. Implement a simple background timer to automatically clean up and delete the sandbox when ttl_seconds is reached.

        self.runtime = SandboxRuntime()
        self.instance_id = self.runtime.create(
            image=image,
            cpu=cpu,
            memory=memory,
            env=env,
            work_dir=work_dir,
            ttl_seconds=ttl_seconds,
            labels=labels,
            timeout_seconds=timeout_seconds,
            rootless=rootless,
            network=network,
            resources=resources,
            readonly=readonly,
            **kwargs,
        )

        if ttl_seconds is not None and ttl_seconds > 0:
            import threading
            self._cleanup_timer = threading.Timer(ttl_seconds, self.delete)
            self._cleanup_timer.daemon = True
            self._cleanup_timer.start()

[cursor]

Whiteout deletion escapes extract dir

High Severity

OCI whiteout handling derives del_name from .wh. prefixes without rejecting . / .. or verifying del_path stays under dest_dir. A crafted image entry such as .wh... makes del_name .., so shutil.rmtree can delete directories outside the extract root, including sibling image caches under /tmp/ray/sandbox/images.

 

^{Reviewed by Cursor Bugbot for commit 8136f7d. Configure here.}

[andrewsykim]

I'm still working through various details, but the PR now reflects what is in the REP and is ready for a first full review. I will address Cursor review shortly.

I used these two scripts to test the changes on Kubernetes and locally:

• eval_code_sandbox.py: runs some determinstic Python code in a sandbox and verifies the output
• eval_tool_sandbox.py: runs some bash script that invokes linux tools with some determinstic outputs

[pcmoritz]

Let's call this workdir everywhere (since both docker and modal call it that), make it optional, and if it is not set, use the container image WORKDIR by default.

[pcmoritz]

This seems unused, let's remove it?

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{Reviewed by Cursor Bugbot for commit 6833afa. Configure here.}

[andrewsykim]

@pcmoritz PR is ready for another review.

Here's how I tested it so far:

• Dockerfile with Ray base image and gVisor installed: https://gist.github.com/andrewsykim/46c6dc45870f1e2fcef5b5f7be6d6257
• RayCluster manifest with minimum security context (priviledged containers not required): https://gist.github.com/andrewsykim/eb9552d369290bf28cdaab8dbe041555
• eval_coding_sandbox.py: https://gist.github.com/andrewsykim/7286c607b7523435303102bf8517d23b
• eval_tool_sandbox.py: https://gist.github.com/andrewsykim/00d1cb20b63b821c725c71eaa231fb13
• eval_unsafe_sandbox.py: https://gist.github.com/andrewsykim/3c82d1b97c02ee71f1cc27cbd2b0f1ed